From 6f7381e42c3787b3b5e70a46b88d8bc181b30c47 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:18:16 +0000 Subject: [PATCH 1/3] Initial plan From b516da332cd8f60ec2e314d14ffc74931f240058 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:34:26 +0000 Subject: [PATCH 2/3] feat: add cross-reference checking for step output fields, SQL aliases, and plain-string step refs Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/70dc13a0-5cff-4fcf-bf94-6417d19991cf --- cmd/wfctl/infra_state.go | 20 +- cmd/wfctl/main.go | 48 ++-- cmd/wfctl/plugin_install_new_test.go | 8 +- cmd/wfctl/template_validate.go | 161 ++++++++++++- cmd/wfctl/template_validate_test.go | 344 +++++++++++++++++++++++++++ cmd/wfctl/test.go | 12 +- 6 files changed, 537 insertions(+), 56 deletions(-) diff --git a/cmd/wfctl/infra_state.go b/cmd/wfctl/infra_state.go index 71503e42..5a2bae6b 100644 --- a/cmd/wfctl/infra_state.go +++ b/cmd/wfctl/infra_state.go @@ -180,11 +180,11 @@ func runInfraStateImport(args []string) error { // --- tfstate export --- type tfState struct { - Version int `json:"version"` - TerraformVersion string `json:"terraform_version"` - Serial int `json:"serial"` - Lineage string `json:"lineage"` - Outputs map[string]any `json:"outputs"` + Version int `json:"version"` + TerraformVersion string `json:"terraform_version"` + Serial int `json:"serial"` + Lineage string `json:"lineage"` + Outputs map[string]any `json:"outputs"` Resources []tfStateResource `json:"resources"` } @@ -302,11 +302,11 @@ func importFromPulumi(srcFile, stateDir string) error { var checkpoint struct { Latest struct { Resources []struct { - URN string `json:"urn"` - Type string `json:"type"` - ID string `json:"id"` - Inputs map[string]any `json:"inputs"` - Outputs map[string]any `json:"outputs"` + URN string `json:"urn"` + Type string `json:"type"` + ID string `json:"id"` + Inputs map[string]any `json:"inputs"` + Outputs map[string]any `json:"outputs"` } `json:"resources"` } `json:"latest"` } diff --git a/cmd/wfctl/main.go b/cmd/wfctl/main.go index c572987b..08442460 100644 --- a/cmd/wfctl/main.go +++ b/cmd/wfctl/main.go @@ -53,30 +53,30 @@ func isHelpRequested(err error) bool { // the runtime functions that are registered in the CLICommandRegistry service // and invoked by step.cli_invoke from within each command's pipeline. var commands = map[string]func([]string) error{ - "init": runInit, - "validate": runValidate, - "inspect": runInspect, - "run": runRun, - "plugin": runPlugin, - "pipeline": runPipeline, - "schema": runSchema, - "snippets": runSnippets, - "manifest": runManifest, - "migrate": runMigrate, - "build-ui": runBuildUI, - "ui": runUI, - "publish": runPublish, - "deploy": runDeploy, - "api": runAPI, - "diff": runDiff, - "template": runTemplate, - "contract": runContract, - "compat": runCompat, - "generate": runGenerate, - "git": runGit, - "registry": runRegistry, - "update": runUpdate, - "mcp": runMCP, + "init": runInit, + "validate": runValidate, + "inspect": runInspect, + "run": runRun, + "plugin": runPlugin, + "pipeline": runPipeline, + "schema": runSchema, + "snippets": runSnippets, + "manifest": runManifest, + "migrate": runMigrate, + "build-ui": runBuildUI, + "ui": runUI, + "publish": runPublish, + "deploy": runDeploy, + "api": runAPI, + "diff": runDiff, + "template": runTemplate, + "contract": runContract, + "compat": runCompat, + "generate": runGenerate, + "git": runGit, + "registry": runRegistry, + "update": runUpdate, + "mcp": runMCP, "modernize": runModernize, "infra": runInfra, "docs": runDocs, diff --git a/cmd/wfctl/plugin_install_new_test.go b/cmd/wfctl/plugin_install_new_test.go index f291f580..cf9d4da0 100644 --- a/cmd/wfctl/plugin_install_new_test.go +++ b/cmd/wfctl/plugin_install_new_test.go @@ -24,8 +24,8 @@ func buildPluginTarGz(t *testing.T, pluginName string, binaryContent []byte, pjC t.Helper() topDir := pluginName + "-" + runtime.GOOS + "-" + runtime.GOARCH entries := map[string][]byte{ - topDir + "/" + pluginName: binaryContent, - topDir + "/plugin.json": pjContent, + topDir + "/" + pluginName: binaryContent, + topDir + "/plugin.json": pjContent, } return buildTarGz(t, entries, 0755) } @@ -166,8 +166,8 @@ func TestInstallFromURL_NameNormalization(t *testing.T) { pjContent := minimalPluginJSON(fullName, "0.1.0") entries := map[string][]byte{ - "top/" + fullName: []byte("#!/bin/sh\n"), - "top/plugin.json": pjContent, + "top/" + fullName: []byte("#!/bin/sh\n"), + "top/plugin.json": pjContent, } tarball := buildTarGz(t, entries, 0755) diff --git a/cmd/wfctl/template_validate.go b/cmd/wfctl/template_validate.go index 97bf929d..8f9afc55 100644 --- a/cmd/wfctl/template_validate.go +++ b/cmd/wfctl/template_validate.go @@ -565,8 +565,10 @@ var _ fs.FS = templateFS // templateExprRe matches template actions {{ ... }}. var templateExprRe = regexp.MustCompile(`\{\{(.*?)\}\}`) -// stepRefDotRe matches .steps.STEP_NAME patterns (dot access). -var stepRefDotRe = regexp.MustCompile(`\.steps\.([a-zA-Z_][a-zA-Z0-9_-]*)`) +// stepRefDotRe matches .steps.STEP_NAME and captures an optional field path. +// Group 1: step name (may contain hyphens). +// 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_]*)*)`) // stepRefIndexRe matches index .steps "STEP_NAME" patterns. var stepRefIndexRe = regexp.MustCompile(`index\s+\.steps\s+"([^"]+)"`) @@ -579,11 +581,44 @@ var stepRefFuncRe = regexp.MustCompile(`(?:^|\||\()\s*step\s+"([^"]+)"`) // 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_-]*)*`) +// 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_]*)*)`) + +// stepBuildInfo holds the type and config of a pipeline step, used for output field validation. +type stepBuildInfo struct { + stepType string + stepConfig map[string]any +} + +// dbQueryStepTypes is the set of step types that produce a "row" or "rows" output +// from a SQL query and support SQL alias extraction. +var dbQueryStepTypes = map[string]bool{ + "step.db_query": true, + "step.db_query_cached": true, +} + +// isDBQueryStep reports whether a step type is a DB query step. +func isDBQueryStep(t string) bool { return dbQueryStepTypes[t] } + +// joinOutputKeys returns a comma-joined list of output key names for error messages. +func joinOutputKeys(outputs []schema.InferredOutput) string { + keys := make([]string, len(outputs)) + for i, o := range outputs { + keys[i] = o.Key + } + return strings.Join(keys, ", ") +} + // validatePipelineTemplates checks template expressions in pipeline step configs for // references to nonexistent or forward-declared steps and common template pitfalls. func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *templateValidationResult) { - // Build ordered step name list - stepNames := make(map[string]int) // step name -> index in pipeline + // 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 + + reg := schema.NewStepSchemaRegistry() + for i, stepRaw := range stepsRaw { stepMap, ok := stepRaw.(map[string]any) if !ok { @@ -592,6 +627,12 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp 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} } } @@ -624,25 +665,29 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp continue } - // Check for step name references via dot-access + // Check for step name references via dot-access (captures optional field path) dotMatches := stepRefDotRe.FindAllStringSubmatch(actionContent, -1) for _, m := range dotMatches { refName := m[1] - validateStepRef(pipelineName, stepName, refName, i, stepNames, result) + fieldPath := "" + if len(m) > 2 { + fieldPath = m[2] + } + validateStepRef(pipelineName, stepName, refName, fieldPath, i, stepNames, stepInfos, reg, result) } - // Check for step name references via index + // Check for step name references via index (no field path resolvable) indexMatches := stepRefIndexRe.FindAllStringSubmatch(actionContent, -1) for _, m := range indexMatches { refName := m[1] - validateStepRef(pipelineName, stepName, refName, i, stepNames, result) + validateStepRef(pipelineName, stepName, refName, "", i, stepNames, stepInfos, reg, result) } - // Check for step name references via step function + // Check for step name references via step function (no field path resolvable) funcMatches := stepRefFuncRe.FindAllStringSubmatch(actionContent, -1) for _, m := range funcMatches { refName := m[1] - validateStepRef(pipelineName, stepName, refName, i, stepNames, result) + validateStepRef(pipelineName, stepName, refName, "", i, stepNames, stepInfos, reg, result) } // Warn on hyphenated dot-access (auto-fixed but suggest preferred syntax) @@ -652,23 +697,115 @@ func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *temp } } } + + // Validate plain-string step references in specific config fields + // (e.g. secret_from, backend_url_key, field in conditional/branch). + if stepCfg, ok := stepMap["config"].(map[string]any); ok { + validatePlainStepRefs(pipelineName, stepName, i, stepCfg, stepNames, stepInfos, reg, result) + } } } // validateStepRef checks that a referenced step name exists and appears before the -// current step in the pipeline execution order. -func validateStepRef(pipelineName, currentStep, refName string, currentIdx int, stepNames map[string]int, result *templateValidationResult) { +// 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 +// for db_query steps it performs best-effort SQL alias checking for "row." paths. +func validateStepRef(pipelineName, currentStep, refName, fieldPath string, currentIdx int, stepNames map[string]int, stepInfos map[string]stepBuildInfo, reg *schema.StepSchemaRegistry, result *templateValidationResult) { refIdx, exists := stepNames[refName] switch { case !exists: result.Warnings = append(result.Warnings, fmt.Sprintf("pipeline %q step %q: references step %q which does not exist in this pipeline", pipelineName, currentStep, refName)) + return case refIdx == currentIdx: result.Warnings = append(result.Warnings, fmt.Sprintf("pipeline %q step %q: references itself; a step cannot use its own outputs because they are not available until after execution", pipelineName, currentStep)) + return case refIdx > currentIdx: result.Warnings = append(result.Warnings, fmt.Sprintf("pipeline %q step %q: references step %q which has not executed yet (appears later in pipeline)", pipelineName, currentStep, refName)) + return + } + + // Step exists and precedes the current step — validate the output field path. + if fieldPath == "" { + return + } + + info, ok := stepInfos[refName] + if !ok || info.stepType == "" { + return + } + + outputs := reg.InferStepOutputs(info.stepType, info.stepConfig) + if len(outputs) == 0 { + return // no schema information available; skip + } + + // Split ".row.auth_token" → ["row", "auth_token"] + parts := strings.Split(strings.TrimPrefix(fieldPath, "."), ".") + if len(parts) == 0 || parts[0] == "" { + return + } + firstField := parts[0] + + // Check the first field against known output keys. + var matchedOutput *schema.InferredOutput + for i := range outputs { + if outputs[i].Key == firstField { + matchedOutput = &outputs[i] + break + } + } + if matchedOutput == nil { + result.Warnings = append(result.Warnings, + fmt.Sprintf("pipeline %q step %q: references step %q output field %q which is not a known output of step type %q (known outputs: %s)", + pipelineName, currentStep, refName, firstField, info.stepType, joinOutputKeys(outputs))) + return + } + + // For db_query/db_query_cached steps, try SQL alias validation on "row." paths. + if firstField == "row" && len(parts) > 1 && isDBQueryStep(info.stepType) { + columnName := parts[1] + query, _ := info.stepConfig["query"].(string) + if query != "" { + sqlCols := extractSQLColumns(query) + if len(sqlCols) > 0 { + found := false + for _, col := range sqlCols { + if col == columnName { + found = true + break + } + } + if !found { + result.Warnings = append(result.Warnings, + fmt.Sprintf("pipeline %q step %q: references step %q output field \"row.%s\" but the SQL query does not select column %q (available: %s)", + pipelineName, currentStep, refName, columnName, columnName, strings.Join(sqlCols, ", "))) + } + } + } + } +} + +// validatePlainStepRefs checks plain-string config values that contain bare step +// context-key references (e.g. "steps.STEP_NAME.field") in config fields known to +// accept such paths: secret_from, backend_url_key, and field (conditional/branch). +func validatePlainStepRefs(pipelineName, stepName string, stepIdx int, stepCfg map[string]any, stepNames map[string]int, stepInfos map[string]stepBuildInfo, reg *schema.StepSchemaRegistry, result *templateValidationResult) { + // Config keys that are documented to accept a bare "steps.X.y" context path. + plainRefKeys := []string{"secret_from", "backend_url_key", "field"} + for _, key := range plainRefKeys { + val, ok := stepCfg[key].(string) + if !ok || val == "" { + continue + } + m := plainStepPathRe.FindStringSubmatch(val) + if m == nil { + continue + } + refName := m[1] + fieldPath := m[2] // already in ".field.subfield" form + validateStepRef(pipelineName, stepName, refName, fieldPath, stepIdx, stepNames, stepInfos, reg, result) } } diff --git a/cmd/wfctl/template_validate_test.go b/cmd/wfctl/template_validate_test.go index 049c5dff..c2a2e898 100644 --- a/cmd/wfctl/template_validate_test.go +++ b/cmd/wfctl/template_validate_test.go @@ -576,3 +576,347 @@ func TestValidateConfigWithHyphenDotAccess(t *testing.T) { t.Errorf("expected informational warning about hyphenated dot-access, got warnings: %v", result.Warnings) } } + +// --- Output field name validation tests --- + +// TestValidateStepOutputField_KnownField checks that a reference to a known output +// field does NOT produce a warning. +func TestValidateStepOutputField_KnownField(t *testing.T) { + 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{"database": "db", "query": "SELECT id FROM users", "mode": "single"}, + }, + map[string]any{ + "name": "respond", + "type": "step.set", + "config": map[string]any{ + "values": map[string]any{ + // "found" is a known output of step.db_query single mode + "ok": "{{ .steps.query.found }}", + }, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "found") && strings.Contains(w, "not a known output") { + t.Errorf("unexpected warning about known output field 'found': %s", w) + } + } +} + +// TestValidateStepOutputField_UnknownField checks that a reference to an unknown +// output field produces a warning. +func TestValidateStepOutputField_UnknownField(t *testing.T) { + 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{"database": "db", "query": "SELECT id FROM users", "mode": "single"}, + }, + map[string]any{ + "name": "respond", + "type": "step.set", + "config": map[string]any{ + "values": map[string]any{ + // "nonexistent_column" is NOT an output of step.db_query + "x": "{{ .steps.query.nonexistent_column }}", + }, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "nonexistent_column") && strings.Contains(w, "not a known output") { + found = true + break + } + } + if !found { + t.Errorf("expected warning about unknown output field 'nonexistent_column', got warnings: %v", result.Warnings) + } +} + +// TestValidateStepOutputField_SQLAlias_Valid checks that a reference to a SQL column +// alias that IS present in the query does NOT produce a warning. +func TestValidateStepOutputField_SQLAlias_Valid(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "load", + "type": "step.db_query", + "config": map[string]any{ + "database": "db", + "query": "SELECT auth_token, affiliate_id FROM integrations WHERE id = $1", + "mode": "single", + }, + }, + map[string]any{ + "name": "verify", + "type": "step.set", + "config": map[string]any{ + "values": map[string]any{ + // auth_token IS selected by the SQL query + "tok": "{{ .steps.load.row.auth_token }}", + }, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "auth_token") && strings.Contains(w, "does not select") { + t.Errorf("unexpected SQL alias warning for known alias 'auth_token': %s", w) + } + } +} + +// TestValidateStepOutputField_SQLAlias_Invalid checks that a reference to a SQL +// column alias that is NOT present in the query produces a warning. +func TestValidateStepOutputField_SQLAlias_Invalid(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "load", + "type": "step.db_query", + "config": map[string]any{ + "database": "db", + // Query selects "auth_token" but not "token" + "query": "SELECT auth_token FROM integrations WHERE id = $1", + "mode": "single", + }, + }, + map[string]any{ + "name": "verify", + "type": "step.set", + "config": map[string]any{ + "values": map[string]any{ + // "token" is NOT a column in the query + "tok": "{{ .steps.load.row.token }}", + }, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "token") && strings.Contains(w, "does not select") { + found = true + break + } + } + if !found { + t.Errorf("expected SQL alias warning for missing column 'token', got warnings: %v", result.Warnings) + } +} + +// TestValidatePlainStepRef_SecretFrom checks that a secret_from value referencing +// a nonexistent step produces a warning. +func TestValidatePlainStepRef_SecretFrom(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "verify", + "type": "step.webhook_verify", + "config": map[string]any{ + // references a step "load" that does not exist in this pipeline + "secret_from": "steps.load.row.auth_token", + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "load") && strings.Contains(w, "does not exist") { + found = true + break + } + } + if !found { + t.Errorf("expected warning about nonexistent step in secret_from, got warnings: %v", result.Warnings) + } +} + +// TestValidatePlainStepRef_SecretFrom_Valid checks that a valid secret_from reference +// pointing to a known step output does NOT produce a warning. +func TestValidatePlainStepRef_SecretFrom_Valid(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "load", + "type": "step.db_query", + "config": map[string]any{ + "database": "db", + "query": "SELECT auth_token FROM integrations WHERE id = $1", + "mode": "single", + }, + }, + map[string]any{ + "name": "verify", + "type": "step.webhook_verify", + "config": map[string]any{ + // references load.row — "row" is a known output of step.db_query single mode + "secret_from": "steps.load.row.auth_token", + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "load") && strings.Contains(w, "does not exist") { + t.Errorf("unexpected warning about existing step 'load': %s", w) + } + if strings.Contains(w, "row") && strings.Contains(w, "not a known output") { + t.Errorf("unexpected warning about known output field 'row': %s", w) + } + } +} + +// TestValidatePlainStepRef_ConditionalField checks that a conditional step's +// "field" config value pointing to a nonexistent step output is warned about. +func TestValidatePlainStepRef_ConditionalField(t *testing.T) { + 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{"database": "db", "query": "SELECT id FROM t", "mode": "single"}, + }, + map[string]any{ + "name": "route", + "type": "step.conditional", + "config": map[string]any{ + // "found" is a known output of step.db_query single mode + "field": "steps.query.found", + "routes": map[string]any{"true": "a"}, + "default": "b", + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "found") && strings.Contains(w, "not a known output") { + t.Errorf("unexpected warning about known output field 'found' in conditional field: %s", w) + } + if strings.Contains(w, "query") && strings.Contains(w, "does not exist") { + t.Errorf("unexpected 'does not exist' warning for existing step 'query': %s", w) + } + } +} + +// TestValidatePlainStepRef_ConditionalField_Unknown verifies that a conditional +// step's "field" referencing a step output that doesn't exist emits a warning. +func TestValidatePlainStepRef_ConditionalField_Unknown(t *testing.T) { + 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{"database": "db", "query": "SELECT id FROM t", "mode": "single"}, + }, + map[string]any{ + "name": "route", + "type": "step.conditional", + "config": map[string]any{ + // "nonexistent_output" is NOT an output of step.db_query + "field": "steps.query.nonexistent_output", + "routes": map[string]any{"true": "a"}, + "default": "b", + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + found := false + for _, w := range result.Warnings { + if strings.Contains(w, "nonexistent_output") && strings.Contains(w, "not a known output") { + found = true + break + } + } + if !found { + t.Errorf("expected warning about unknown output field in conditional field, got warnings: %v", result.Warnings) + } +} diff --git a/cmd/wfctl/test.go b/cmd/wfctl/test.go index eeb83839..f166d153 100644 --- a/cmd/wfctl/test.go +++ b/cmd/wfctl/test.go @@ -243,9 +243,9 @@ type testMockConfig struct { } type testCase struct { - Description string `yaml:"description"` - Trigger testTriggerDef `yaml:"trigger"` - StopAfter string `yaml:"stop_after"` + Description string `yaml:"description"` + Trigger testTriggerDef `yaml:"trigger"` + StopAfter string `yaml:"stop_after"` Mocks *testMockConfig `yaml:"mocks"` Assertions []testAssertion `yaml:"assertions"` } @@ -260,9 +260,9 @@ type testTriggerDef struct { } type testAssertion struct { - Step string `yaml:"step"` - Output map[string]any `yaml:"output"` - Executed *bool `yaml:"executed"` + Step string `yaml:"step"` + Output map[string]any `yaml:"output"` + Executed *bool `yaml:"executed"` Response *testResponseAssert `yaml:"response"` } From 8fe8a0b01ea09e6a04e676fcb472b98b98bc97c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 17:45:16 +0000 Subject: [PATCH 3/3] fix: skip field-path validation for steps with dynamic/wildcard placeholder outputs Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/bccc8889-cac7-4453-9778-c5596f9372b9 --- cmd/wfctl/template_validate.go | 37 ++++++++++++++++++++--- cmd/wfctl/template_validate_test.go | 46 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/cmd/wfctl/template_validate.go b/cmd/wfctl/template_validate.go index 8f9afc55..df49147c 100644 --- a/cmd/wfctl/template_validate.go +++ b/cmd/wfctl/template_validate.go @@ -601,15 +601,37 @@ var dbQueryStepTypes = map[string]bool{ // isDBQueryStep reports whether a step type is a DB query step. func isDBQueryStep(t string) bool { return dbQueryStepTypes[t] } -// joinOutputKeys returns a comma-joined list of output key names for error messages. +// joinOutputKeys returns a comma-joined list of output key names for error messages, +// omitting placeholder/wildcard entries like "(key)", "(dynamic)", "(nested)". func joinOutputKeys(outputs []schema.InferredOutput) string { - keys := make([]string, len(outputs)) - for i, o := range outputs { - keys[i] = o.Key + keys := make([]string, 0, len(outputs)) + for _, o := range outputs { + if !isPlaceholderOutputKey(o.Key) { + keys = append(keys, o.Key) + } } return strings.Join(keys, ", ") } +// isPlaceholderOutputKey reports whether an output key is a dynamic/wildcard +// placeholder (e.g. "(key)", "(dynamic)", "(nested)"). Steps that expose +// such placeholders produce outputs whose field names cannot be statically +// determined, so field-path validation should be skipped for them. +func isPlaceholderOutputKey(key string) bool { + return len(key) >= 2 && key[0] == '(' && key[len(key)-1] == ')' +} + +// hasDynamicOutputs reports whether any output in the list is a wildcard +// placeholder, meaning the step emits fields that are not statically known. +func hasDynamicOutputs(outputs []schema.InferredOutput) bool { + for _, o := range outputs { + if isPlaceholderOutputKey(o.Key) { + return true + } + } + return false +} + // validatePipelineTemplates checks template expressions in pipeline step configs for // references to nonexistent or forward-declared steps and common template pitfalls. func validatePipelineTemplates(pipelineName string, stepsRaw []any, result *templateValidationResult) { @@ -742,6 +764,13 @@ func validateStepRef(pipelineName, currentStep, refName, fieldPath string, curre return // no schema information available; skip } + // If any output key is a placeholder (e.g. "(key)", "(dynamic)", "(nested)"), + // the step emits dynamic fields whose names cannot be statically determined. + // Skip field-path validation for such steps to avoid false positives. + if hasDynamicOutputs(outputs) { + return + } + // Split ".row.auth_token" → ["row", "auth_token"] parts := strings.Split(strings.TrimPrefix(fieldPath, "."), ".") if len(parts) == 0 || parts[0] == "" { diff --git a/cmd/wfctl/template_validate_test.go b/cmd/wfctl/template_validate_test.go index c2a2e898..a6f17057 100644 --- a/cmd/wfctl/template_validate_test.go +++ b/cmd/wfctl/template_validate_test.go @@ -920,3 +920,49 @@ func TestValidatePlainStepRef_ConditionalField_Unknown(t *testing.T) { t.Errorf("expected warning about unknown output field in conditional field, got warnings: %v", result.Warnings) } } + +// TestValidateStepOutputField_DynamicOutputSkipped verifies that steps with +// dynamic/wildcard placeholder outputs (e.g. "(key)" from step.secret_fetch, +// "(dynamic)" from step.set) do NOT generate false-positive warnings when +// arbitrary field names are accessed. +func TestValidateStepOutputField_DynamicOutputSkipped(t *testing.T) { + cfg := &config.WorkflowConfig{ + Pipelines: map[string]any{ + "api": map[string]any{ + "steps": []any{ + map[string]any{ + "name": "fetch", + "type": "step.secret_fetch", + "config": map[string]any{ + "secrets": map[string]any{ + "api_key": "env://API_KEY", + }, + }, + }, + map[string]any{ + "name": "call", + "type": "step.http_call", + "config": map[string]any{ + // "api_key" is a dynamic output of step.secret_fetch (not statically declared) + "url": "https://api.example.com", + "headers": map[string]any{ + "Authorization": "Bearer {{ .steps.fetch.api_key }}", + }, + }, + }, + }, + }, + }, + } + knownModules := KnownModuleTypes() + knownSteps := KnownStepTypes() + knownTriggers := KnownTriggerTypes() + + result := validateWorkflowConfig("test", cfg, knownModules, knownSteps, knownTriggers) + + for _, w := range result.Warnings { + if strings.Contains(w, "api_key") && strings.Contains(w, "not a known output") { + t.Errorf("unexpected false-positive warning about dynamic output field 'api_key': %s", w) + } + } +}