diff --git a/cmd/wfctl/contract.go b/cmd/wfctl/contract.go index dc059d8a..51a200bf 100644 --- a/cmd/wfctl/contract.go +++ b/cmd/wfctl/contract.go @@ -26,10 +26,11 @@ type Contract struct { // EndpointContract describes an HTTP endpoint in the contract. type EndpointContract struct { - Method string `json:"method"` - Path string `json:"path"` - AuthRequired bool `json:"authRequired"` - Pipeline string `json:"pipeline"` + Method string `json:"method"` + Path string `json:"path"` + AuthRequired bool `json:"authRequired"` + Pipeline string `json:"pipeline"` + ResponseSchema map[string]string `json:"responseSchema,omitempty"` // field name → type } // ModuleContract describes a module in the contract. @@ -268,6 +269,8 @@ func generateContract(cfg *config.WorkflowConfig) *Contract { } } } + // Populate response schema from pipeline outputs declaration + ep.ResponseSchema = extractPipelineOutputSchema(pipelineMap) contract.Endpoints = append(contract.Endpoints, ep) } } @@ -343,6 +346,31 @@ func generateContract(cfg *config.WorkflowConfig) *Contract { return contract } +// extractPipelineOutputSchema reads the optional "outputs" block from a raw +// pipeline map and returns a field→type map for use in EndpointContract. +// Returns nil when the pipeline has no outputs declaration. +func extractPipelineOutputSchema(pipelineMap map[string]any) map[string]string { + outputsRaw, ok := pipelineMap["outputs"] + if !ok || outputsRaw == nil { + return nil + } + outputsMap, ok := outputsRaw.(map[string]any) + if !ok || len(outputsMap) == 0 { + return nil + } + schema := make(map[string]string, len(outputsMap)) + for field, defRaw := range outputsMap { + fieldType := "any" + if defMap, ok := defRaw.(map[string]any); ok { + if t, ok := defMap["type"].(string); ok && t != "" { + fieldType = t + } + } + schema[field] = fieldType + } + return schema +} + // compareContracts compares a baseline contract to the current one. func compareContracts(base, current *Contract) *contractComparison { comp := &contractComparison{ @@ -365,18 +393,37 @@ func compareContracts(base, current *Contract) *contractComparison { // Check base endpoints for key, baseEP := range baseEPs { if currentEP, exists := currentEPs[key]; exists { - // Check for breaking changes + // Collect all breaking changes for this endpoint + var breakingDetails []string + + // Auth was added to a public endpoint if baseEP.AuthRequired != currentEP.AuthRequired && !baseEP.AuthRequired { - // Auth was added to a public endpoint - comp.Endpoints = append(comp.Endpoints, endpointChange{ - Method: baseEP.Method, - Path: baseEP.Path, - Pipeline: currentEP.Pipeline, - Change: changeChanged, - Detail: "auth requirement added (clients without tokens will get 401)", - IsBreaking: true, - }) - comp.BreakingCount++ + breakingDetails = append(breakingDetails, "auth requirement added (clients without tokens will get 401)") + } + + // Response schema fields were removed or changed type + for field, baseType := range baseEP.ResponseSchema { + if currentType, exists := currentEP.ResponseSchema[field]; !exists { + breakingDetails = append(breakingDetails, + fmt.Sprintf("response field %q removed (was %s)", field, baseType)) + } else if baseType != currentType && baseType != "any" && currentType != "any" { + breakingDetails = append(breakingDetails, + fmt.Sprintf("response field %q changed type from %s to %s", field, baseType, currentType)) + } + } + + if len(breakingDetails) > 0 { + for _, detail := range breakingDetails { + comp.Endpoints = append(comp.Endpoints, endpointChange{ + Method: baseEP.Method, + Path: baseEP.Path, + Pipeline: currentEP.Pipeline, + Change: changeChanged, + Detail: detail, + IsBreaking: true, + }) + comp.BreakingCount++ + } } else { comp.Endpoints = append(comp.Endpoints, endpointChange{ Method: baseEP.Method, @@ -511,6 +558,17 @@ func printContract(c *Contract) { auth = " [auth]" } fmt.Printf(" %-7s %s%s (pipeline: %s)\n", ep.Method, ep.Path, auth, ep.Pipeline) + if len(ep.ResponseSchema) > 0 { + // Print response schema fields sorted for stable output + fields := make([]string, 0, len(ep.ResponseSchema)) + for f := range ep.ResponseSchema { + fields = append(fields, f) + } + sort.Strings(fields) + for _, f := range fields { + fmt.Printf(" response.%s: %s\n", f, ep.ResponseSchema[f]) + } + } } fmt.Printf("\nModules (%d):\n", len(c.Modules)) diff --git a/cmd/wfctl/contract_test.go b/cmd/wfctl/contract_test.go index 61e81706..0c272bb6 100644 --- a/cmd/wfctl/contract_test.go +++ b/cmd/wfctl/contract_test.go @@ -560,3 +560,235 @@ pipelines: t.Errorf("expected 'breaking' in error message, got: %v", err) } } + +// TestGenerateContractResponseSchema checks that the response schema is extracted +// from the pipeline's optional outputs declaration. +func TestGenerateContractResponseSchema(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "get-item": map[string]any{ + "trigger": map[string]any{ + "type": "http", + "config": map[string]any{ + "path": "/api/items/:id", + "method": "GET", + }, + }, + "outputs": map[string]any{ + "id": map[string]any{"type": "string"}, + "name": map[string]any{"type": "string"}, + "found": map[string]any{"type": "boolean"}, + }, + "steps": []any{}, + }, + }, + } + + contract := generateContract(cfg) + + if len(contract.Endpoints) != 1 { + t.Fatalf("expected 1 endpoint, got %d", len(contract.Endpoints)) + } + ep := contract.Endpoints[0] + if len(ep.ResponseSchema) != 3 { + t.Fatalf("expected 3 response schema fields, got %d: %v", len(ep.ResponseSchema), ep.ResponseSchema) + } + if ep.ResponseSchema["id"] != "string" { + t.Errorf("expected id=string, got %q", ep.ResponseSchema["id"]) + } + if ep.ResponseSchema["found"] != "boolean" { + t.Errorf("expected found=boolean, got %q", ep.ResponseSchema["found"]) + } +} + +// TestGenerateContractNoResponseSchema checks that endpoints without an outputs +// block have a nil ResponseSchema (backwards-compatible). +func TestGenerateContractNoResponseSchema(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "get-item": map[string]any{ + "trigger": map[string]any{ + "type": "http", + "config": map[string]any{ + "path": "/api/items", + "method": "GET", + }, + }, + "steps": []any{}, + }, + }, + } + + contract := generateContract(cfg) + if len(contract.Endpoints) != 1 { + t.Fatalf("expected 1 endpoint, got %d", len(contract.Endpoints)) + } + if contract.Endpoints[0].ResponseSchema != nil { + t.Errorf("expected nil ResponseSchema for pipeline without outputs, got %v", contract.Endpoints[0].ResponseSchema) + } +} + +// TestCompareContracts_ResponseSchemaFieldRemoved checks that removing a +// declared response schema field is detected as a breaking change. +func TestCompareContracts_ResponseSchemaFieldRemoved(t *testing.T) { + base := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{ + "id": "string", + "name": "string", + "found": "boolean", + }, + }, + }, + } + current := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{ + "id": "string", + "name": "string", + // "found" has been removed + }, + }, + }, + } + + comp := compareContracts(base, current) + + if comp.BreakingCount == 0 { + t.Error("expected breaking change when response field is removed") + } + found := false + for _, ec := range comp.Endpoints { + if ec.IsBreaking && strings.Contains(ec.Detail, "found") && strings.Contains(ec.Detail, "removed") { + found = true + break + } + } + if !found { + t.Errorf("expected breaking change about removed field 'found', got: %v", comp.Endpoints) + } +} + +// TestCompareContracts_ResponseSchemaTypeChanged checks that changing a +// declared response schema field type is detected as a breaking change. +func TestCompareContracts_ResponseSchemaTypeChanged(t *testing.T) { + base := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{ + "count": "integer", + }, + }, + }, + } + current := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{ + "count": "string", // changed from integer to string + }, + }, + }, + } + + comp := compareContracts(base, current) + + if comp.BreakingCount == 0 { + t.Error("expected breaking change when response field type changes") + } + found := false + for _, ec := range comp.Endpoints { + if ec.IsBreaking && strings.Contains(ec.Detail, "count") && strings.Contains(ec.Detail, "changed type") { + found = true + break + } + } + if !found { + t.Errorf("expected breaking change about type change for 'count', got: %v", comp.Endpoints) + } +} + +// TestCompareContracts_ResponseSchemaFieldAdded checks that adding a new +// response schema field is not a breaking change. +func TestCompareContracts_ResponseSchemaFieldAdded(t *testing.T) { + base := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{"id": "string"}, + }, + }, + } + current := &Contract{ + Version: "1.0", + Endpoints: []EndpointContract{ + { + Method: "GET", + Path: "/api/items", + Pipeline: "get-items", + ResponseSchema: map[string]string{"id": "string", "name": "string"}, + }, + }, + } + + comp := compareContracts(base, current) + + if comp.BreakingCount != 0 { + t.Errorf("expected no breaking changes when a new field is added, got %d: %v", comp.BreakingCount, comp.Endpoints) + } +} + +// TestExtractPipelineOutputSchema tests the helper that reads the outputs block. +func TestExtractPipelineOutputSchema(t *testing.T) { + t.Run("with outputs", func(t *testing.T) { + pipelineMap := map[string]any{ + "outputs": map[string]any{ + "id": map[string]any{"type": "string", "description": "Item ID"}, + "found": map[string]any{"type": "boolean"}, + "score": map[string]any{}, // missing type → defaults to "any" + }, + } + got := extractPipelineOutputSchema(pipelineMap) + if got == nil { + t.Fatal("expected non-nil schema") + } + if got["id"] != "string" { + t.Errorf("id: want string, got %q", got["id"]) + } + if got["found"] != "boolean" { + t.Errorf("found: want boolean, got %q", got["found"]) + } + if got["score"] != "any" { + t.Errorf("score: want any, got %q", got["score"]) + } + }) + + t.Run("without outputs", func(t *testing.T) { + pipelineMap := map[string]any{"steps": []any{}} + got := extractPipelineOutputSchema(pipelineMap) + if got != nil { + t.Errorf("expected nil for pipeline without outputs, got %v", got) + } + }) +} diff --git a/cmd/wfctl/template_validate.go b/cmd/wfctl/template_validate.go index df49147c..18f0a997 100644 --- a/cmd/wfctl/template_validate.go +++ b/cmd/wfctl/template_validate.go @@ -570,6 +570,9 @@ var templateExprRe = regexp.MustCompile(`\{\{(.*?)\}\}`) // Group 2: remaining dot-path (e.g. ".row.auth_token"), field names without hyphens. var stepRefDotRe = regexp.MustCompile(`\.steps\.([a-zA-Z_][a-zA-Z0-9_-]*)((?:\.[a-zA-Z_][a-zA-Z0-9_]*)*)`) +// stepFieldDotRe matches .steps.STEP_NAME.FIELD_NAME (captures step and first field). +var stepFieldDotRe = regexp.MustCompile(`\.steps\.([a-zA-Z_][a-zA-Z0-9_-]*)\.([a-zA-Z_][a-zA-Z0-9_-]*)`) + // stepRefIndexRe matches index .steps "STEP_NAME" patterns. var stepRefIndexRe = regexp.MustCompile(`index\s+\.steps\s+"([^"]+)"`) @@ -577,10 +580,21 @@ var stepRefIndexRe = regexp.MustCompile(`index\s+\.steps\s+"([^"]+)"`) // action, after a pipe, or after an opening parenthesis. var stepRefFuncRe = regexp.MustCompile(`(?:^|\||\()\s*step\s+"([^"]+)"`) +// stepFuncFieldRe matches step "STEP_NAME" "FIELD_NAME" capturing both arguments, +// when used as a function call at the start of an action, after a pipe, or after +// an opening parenthesis. +var stepFuncFieldRe = regexp.MustCompile(`(?:^|\||\()\s*step\s+"([^"]+)"\s+"([^"]+)"`) + // hyphenDotRe matches dot-access chains with hyphens (e.g., .steps.my-step.field), // including continuation segments after the hyphenated part. var hyphenDotRe = regexp.MustCompile(`\.[a-zA-Z_][a-zA-Z0-9_]*-[a-zA-Z0-9_-]*(?:\.[a-zA-Z_][a-zA-Z0-9_-]*)*`) +// pipelineStepMeta holds the type and config of a pipeline step for static analysis. +type pipelineStepMeta struct { + typ string + config map[string]any +} + // plainStepPathRe matches bare step context-key references such as // "steps.STEP_NAME.field.subfield" used in plain-string config values (no {{ }}). var plainStepPathRe = regexp.MustCompile(`^steps\.([a-zA-Z_][a-zA-Z0-9_-]*)((?:\.[a-zA-Z_][a-zA-Z0-9_]*)*)`) @@ -634,10 +648,13 @@ func hasDynamicOutputs(outputs []schema.InferredOutput) bool { // validatePipelineTemplates checks template expressions in pipeline step configs for // references to nonexistent or forward-declared steps and common template pitfalls. +// It also warns when a template references a field that is not in the step type's +// declared output schema (Phase 1 static analysis). func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *templateValidationResult) { - // Build ordered step name list and per-step type/config info. - stepNames := make(map[string]int) // step name -> index in pipeline - stepInfos := make(map[string]stepBuildInfo) // step name -> type and config + // Build ordered step name list and step metadata for schema validation. + stepNames := make(map[string]int) // step name -> index in pipeline + stepMeta := make(map[string]pipelineStepMeta) // step name -> type+config (used by validateStepOutputField) + stepInfos := make(map[string]stepBuildInfo) // step name -> type and config (used by validateStepRef) reg := schema.NewStepSchemaRegistry() @@ -647,15 +664,19 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp continue } name, _ := stepMap["name"].(string) - if name != "" { - stepNames[name] = i - sType, _ := stepMap["type"].(string) - sCfg, _ := stepMap["config"].(map[string]any) - if sCfg == nil { - sCfg = map[string]any{} - } - stepInfos[name] = stepBuildInfo{stepType: sType, stepConfig: sCfg} + if name == "" { + continue + } + + stepNames[name] = i + typ, _ := stepMap["type"].(string) + cfg, _ := stepMap["config"].(map[string]any) + if cfg == nil { + cfg = map[string]any{} } + + stepInfos[name] = stepBuildInfo{stepType: typ, stepConfig: cfg} + stepMeta[name] = pipelineStepMeta{typ: typ, config: cfg} } // Check each step's config for template expressions @@ -698,6 +719,17 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp validateStepRef(pipelineName, stepName, refName, fieldPath, i, stepNames, stepInfos, reg, result) } + // Check for step output field references via dot-access (.steps.NAME.FIELD) + // Skip when the action contains hyphenated dot-access, which is not valid + // Go-template syntax and is already flagged separately by hyphenDotRe. + if !hyphenDotRe.MatchString(actionContent) { + fieldDotMatches := stepFieldDotRe.FindAllStringSubmatch(actionContent, -1) + for _, m := range fieldDotMatches { + refStepName, refField := m[1], m[2] + validateStepOutputField(pipelineName, stepName, refStepName, refField, stepMeta, reg, result) + } + } + // Check for step name references via index (no field path resolvable) indexMatches := stepRefIndexRe.FindAllStringSubmatch(actionContent, -1) for _, m := range indexMatches { @@ -712,6 +744,13 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp validateStepRef(pipelineName, stepName, refName, "", i, stepNames, stepInfos, reg, result) } + // Check for step output field references via step function (step "NAME" "FIELD") + funcFieldMatches := stepFuncFieldRe.FindAllStringSubmatch(actionContent, -1) + for _, m := range funcFieldMatches { + refStepName, refField := m[1], m[2] + validateStepOutputField(pipelineName, stepName, refStepName, refField, stepMeta, reg, result) + } + // Warn on hyphenated dot-access (auto-fixed but suggest preferred syntax) if hyphenDotRe.MatchString(actionContent) { result.Warnings = append(result.Warnings, @@ -728,6 +767,44 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp } } +// validateStepOutputField checks that a referenced output field exists in the +// step type's declared output schema. It emits a warning when the step type is +// known, has declared outputs, and none of them match the referenced field. +func validateStepOutputField(pipelineName, currentStep, refStepName, refField string, stepMeta map[string]pipelineStepMeta, reg *schema.StepSchemaRegistry, result *templateValidationResult) { + meta, ok := stepMeta[refStepName] + if !ok || meta.typ == "" { + return // step name unknown or no type — already caught by validateStepRef + } + + outputs := reg.InferStepOutputs(meta.typ, meta.config) + if len(outputs) == 0 { + return // no declared outputs for this step type — nothing to check + } + + // If any output key is a placeholder (wrapped in parentheses), the step + // has dynamic/config-dependent outputs and we cannot validate statically. + for _, o := range outputs { + if len(o.Key) > 1 && o.Key[0] == '(' && o.Key[len(o.Key)-1] == ')' { + return + } + } + + for _, o := range outputs { + if o.Key == refField { + return // field found — all good + } + } + + // Build a suggestion list from declared outputs + keys := make([]string, 0, len(outputs)) + for _, o := range outputs { + keys = append(keys, o.Key) + } + result.Warnings = append(result.Warnings, + fmt.Sprintf("pipeline %q step %q: references %s.%s but step %q (%s) declares outputs: %s", + pipelineName, currentStep, refStepName, refField, refStepName, meta.typ, strings.Join(keys, ", "))) +} + // validateStepRef checks that a referenced step name exists and appears before the // current step in the pipeline execution order. When fieldPath is non-empty it // also validates the first output field name against the step's known outputs, and diff --git a/cmd/wfctl/template_validate_test.go b/cmd/wfctl/template_validate_test.go index a6f17057..56a817d9 100644 --- a/cmd/wfctl/template_validate_test.go +++ b/cmd/wfctl/template_validate_test.go @@ -577,6 +577,309 @@ func TestValidateConfigWithHyphenDotAccess(t *testing.T) { } } +// TestValidateStepOutputField_UndeclaredField_Warning checks that the validator +// warns when a template references a field that is not in the step type's +// declared output schema (Phase 1 static analysis). +func TestValidateStepOutputField_UndeclaredField_Warning(t *testing.T) { + // step.db_query with mode:single declares outputs: row, found + // Referencing .steps.query.rows (plural) should warn + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "query", + "type": "step.db_query", + "config": map[string]any{ + "db": "mydb", + "query": "SELECT * FROM items WHERE id = 1", + "mode": "single", + }, + }, + map[string]any{ + "name": "respond", + "type": "step.json_response", + "config": map[string]any{ + "status": 200, + // "rows" is the list-mode output; single-mode declares "row" + "body": `{{ json .steps.query.rows }}`, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "rows") && strings.Contains(w, "query") { + found = true + break + } + } + if !found { + t.Errorf("expected warning for undeclared field 'rows' on step.db_query (single mode), got warnings: %v", result.Warnings) + } +} + +// TestValidateStepOutputField_DeclaredField_NoWarning checks that referencing a +// declared output field does not produce a warning. +func TestValidateStepOutputField_DeclaredField_NoWarning(t *testing.T) { + // step.db_query with mode:single declares outputs: row, found + // Referencing .steps.query.row should NOT warn + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "query", + "type": "step.db_query", + "config": map[string]any{ + "db": "mydb", + "query": "SELECT * FROM items WHERE id = 1", + "mode": "single", + }, + }, + map[string]any{ + "name": "respond", + "type": "step.json_response", + "config": map[string]any{ + "status": 200, + "body": `{{ json .steps.query.row }}`, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "query.row") && strings.Contains(w, "declares outputs") { + t.Errorf("unexpected warning for declared field 'row': %s", w) + } + } +} + +// TestValidateStepOutputField_StepFuncSyntax_Warning verifies that the validator +// also checks field names in the step "NAME" "FIELD" function call syntax. +func TestValidateStepOutputField_StepFuncSyntax_Warning(t *testing.T) { + // step.db_query with mode:list declares outputs: rows, count + // Using step "query" "row" (singular) should warn + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "query", + "type": "step.db_query", + "config": map[string]any{ + "mode": "list", + }, + }, + map[string]any{ + "name": "respond", + "type": "step.json_response", + "config": map[string]any{ + "status": 200, + // list mode declares "rows"/"count", not "row" + "body": `{{ step "query" "row" }}`, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "row") && strings.Contains(w, "query") { + found = true + break + } + } + if !found { + t.Errorf("expected warning for undeclared field 'row' on step.db_query (list mode) via step func, got warnings: %v", result.Warnings) + } +} + +// TestValidateStepOutputField_SetStep_NoWarning checks that step.set outputs +// inferred from config are validated correctly (declared key should not warn). +func TestValidateStepOutputField_SetStep_NoWarning(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "setter", + "type": "step.set", + "config": map[string]any{ + "values": map[string]any{ + "user_id": "123", + }, + }, + }, + map[string]any{ + "name": "respond", + "type": "step.json_response", + "config": map[string]any{ + "status": 200, + "body": `{{ .steps.setter.user_id }}`, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "declares outputs") && strings.Contains(w, "setter") { + t.Errorf("unexpected step output warning for step.set with declared key: %s", w) + } + } +} + +// TestValidateStepOutputField_NoOutputSchema_NoWarning checks that steps with +// no declared outputs do not produce false-positive field warnings. +func TestValidateStepOutputField_NoOutputSchema_NoWarning(t *testing.T) { + // An unknown/external step type has no schema — any field reference should be silently ignored + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "external", + "type": "step.external_custom_step", // not in registry + }, + map[string]any{ + "name": "respond", + "type": "step.json_response", + "config": map[string]any{ + "status": 200, + "body": `{{ .steps.external.anything }}`, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "declares outputs") && strings.Contains(w, "external") { + t.Errorf("unexpected output field warning for step with no declared outputs: %s", w) + } + } +} + +// TestValidateStepOutputFieldRegistry tests validateStepOutputField directly. +func TestValidateStepOutputFieldRegistry(t *testing.T) { + reg := schema.NewStepSchemaRegistry() + + tests := []struct { + name string + stepName string + stepType string + stepConfig map[string]any + refField string + expectWarn bool + }{ + { + name: "db_query single mode: valid field row", + stepName: "q", + stepType: "step.db_query", + stepConfig: map[string]any{"mode": "single"}, + refField: "row", + expectWarn: false, + }, + { + name: "db_query single mode: invalid field rows", + stepName: "q", + stepType: "step.db_query", + stepConfig: map[string]any{"mode": "single"}, + refField: "rows", + expectWarn: true, + }, + { + name: "db_query list mode: valid field rows", + stepName: "q", + stepType: "step.db_query", + stepConfig: map[string]any{"mode": "list"}, + refField: "rows", + expectWarn: false, + }, + { + name: "db_query list mode: invalid field row", + stepName: "q", + stepType: "step.db_query", + stepConfig: map[string]any{"mode": "list"}, + refField: "row", + expectWarn: true, + }, + { + name: "step.set: declared key is valid", + stepName: "s", + stepType: "step.set", + stepConfig: map[string]any{"values": map[string]any{"my_field": "v"}}, + refField: "my_field", + expectWarn: false, + }, + { + name: "step.set: undeclared key warns", + stepName: "s", + stepType: "step.set", + stepConfig: map[string]any{"values": map[string]any{"my_field": "v"}}, + refField: "other_field", + expectWarn: true, + }, + { + name: "unknown step type: no warning", + stepName: "s", + stepType: "step.nonexistent", + stepConfig: nil, + refField: "anything", + expectWarn: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := &templateValidationResult{} + stepMeta := map[string]pipelineStepMeta{ + tc.stepName: {typ: tc.stepType, config: tc.stepConfig}, + } + validateStepOutputField("pipeline", "current-step", tc.stepName, tc.refField, stepMeta, reg, result) + hasWarn := len(result.Warnings) > 0 + if hasWarn != tc.expectWarn { + t.Errorf("expectWarn=%v but warnings=%v", tc.expectWarn, result.Warnings) + } + }) + } +} + // --- Output field name validation tests --- // TestValidateStepOutputField_KnownField checks that a reference to a known output diff --git a/config/pipeline.go b/config/pipeline.go index bf9d770e..0650495a 100644 --- a/config/pipeline.go +++ b/config/pipeline.go @@ -1,5 +1,14 @@ package config +// PipelineOutputDef describes a single declared output field of a pipeline. +// When a pipeline declares its outputs, callers (HTTP triggers, +// step.workflow_call, wfctl contract test) can validate or document the +// expected response shape. +type PipelineOutputDef struct { + Type string `json:"type" yaml:"type"` + Description string `json:"description,omitempty" yaml:"description,omitempty"` +} + // PipelineConfig represents a single composable pipeline definition. type PipelineConfig struct { Trigger PipelineTriggerConfig `json:"trigger" yaml:"trigger"` @@ -7,6 +16,14 @@ type PipelineConfig struct { OnError string `json:"on_error,omitempty" yaml:"on_error,omitempty"` Timeout string `json:"timeout,omitempty" yaml:"timeout,omitempty"` Compensation []PipelineStepConfig `json:"compensation,omitempty" yaml:"compensation,omitempty"` + // Outputs declares the named output fields that this pipeline is expected + // to produce. This is an optional, backwards-compatible declaration — + // existing pipelines without an Outputs block continue to work unchanged. + // When present, it enables: + // - wfctl contract test to include response schemas in endpoint contracts + // - tools that understand pipeline outputs (for example, step.workflow_call) + // to validate or document output_mapping and other response shapes + Outputs map[string]PipelineOutputDef `json:"outputs,omitempty" yaml:"outputs,omitempty"` // StrictTemplates causes the pipeline to return an error when any template // expression references a missing map key, instead of silently using the zero // value. Useful for catching typos in step field references at runtime. diff --git a/module/pipeline_step_deploy_rolling.go b/module/pipeline_step_deploy_rolling.go index f7ae122d..8d889819 100644 --- a/module/pipeline_step_deploy_rolling.go +++ b/module/pipeline_step_deploy_rolling.go @@ -242,4 +242,3 @@ func (s *DeployRollingStep) Execute(ctx context.Context, _ *PipelineContext) (*S "batches": totalBatches, }}, nil } -