Conversation
…s, 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
Copilot
AI
changed the title
[WIP] Add cross-reference checking for DB columns and step outputs
wfctl validate: add field-level cross-reference checking for step outputs, SQL aliases, and plain-string step refs
Mar 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances wfctl validate / wfctl template validate by adding static, field-level cross-reference checks for step-output paths found in templates and selected plain-string config fields, with additional best-effort SQL SELECT alias validation for step.db_query-style steps. This reduces runtime-only failures due to typos or renamed fields/aliases.
Changes:
- Extend template step-reference parsing to capture and validate the first output field segment (via
schema.StepSchemaRegistry.InferStepOutputs). - Add best-effort SQL column/alias extraction validation for
steps.<db_query>.row.<col>references. - Validate certain plain-string config fields (
secret_from,backend_url_key,field) using the same step-ref validation logic, and add targeted tests.
Reviewed changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/wfctl/template_validate.go | Adds field-suffix capture to step ref parsing and validates output keys + SQL aliases; introduces plain-string step path validation. |
| cmd/wfctl/template_validate_test.go | Adds test coverage for output-key validation, SQL alias checking, and plain-string step refs. |
| cmd/wfctl/test.go | Formatting-only (gofmt alignment). |
| cmd/wfctl/plugin_install_new_test.go | Formatting-only (gofmt alignment). |
| cmd/wfctl/main.go | Formatting-only (gofmt alignment). |
| cmd/wfctl/infra_state.go | Formatting-only (gofmt alignment). |
Comment on lines
+760
to
+764
| 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 |
There was a problem hiding this comment.
The output-field validation will incorrectly warn for steps whose outputs are intentionally dynamic map keys (e.g. step.secret_fetch exposes arbitrary keys from its secrets map and its schema uses the placeholder output key "(key)"). With the current exact-match check on outputs[i].Key == firstField, any access like .steps.fetch.api_key will be flagged as unknown even though it’s valid. Consider treating placeholder keys like "(key)" as a wildcard (skip validation for that step type) or enhancing output inference for those steps to return the concrete keys from config so validation can be accurate.
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Contributor
|
@copilot apply changes based on the comments in this thread |
…holder 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
Contributor
Author
Fixed in 8fe8a0b. Added |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wfctl validateandwfctl template validatecaught structural errors (unknown types, missing steps) but not field-level reference errors — a renamed SQL alias or wrong output field only failed at runtime.What's added
Step output field validation — when a template accesses
.steps.query.someField, the first field segment is now checked against the step type's known outputs viaschema.InferStepOutputs. Unknown fields emit a warning with the list of valid outputs.SQL alias extraction (best-effort) — for
step.db_query/step.db_query_cachedrefs with arow.<col>path, the SQLSELECTclause is parsed to extract column aliases. A mismatch (e.g. template readsrow.tokenbut query selectsauth_token) emits a warning. Silent onSELECT *and dynamic SQL.Plain-string step ref validation — config fields that accept bare context-key paths (
secret_from,backend_url_key,fieldin conditional/branch steps) are now validated through the samevalidateStepReflogic, catching nonexistent steps and unknown output fields without a template expression.Example
Implementation notes
stepRefDotReextended with a second capture group for the field-path suffix (.row.auth_token)stepBuildInfostruct added to carry per-step type + config throughvalidatePipelineTemplatesvalidateStepRefsignature extended;validatePlainStepRefsadded as a new helperextractSQLColumnsfunction already present inapi_extract.goOriginal prompt
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.