template: warn on missing key access; add strict_templates opt-in#372
Merged
template: warn on missing key access; add strict_templates opt-in#372
Conversation
…rict mode) Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/a9d0671f-a3b4-4962-9915-5e48a9e14c70
Copilot
AI
changed the title
[WIP] Fix template missingkey=zero issue with field-level typos
template: warn on missing key access; add strict_templates opt-in
Mar 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds safer template evaluation for pipelines by surfacing missing-key typos at runtime (WARN by default) and allowing opt-in strict mode (strict_templates: true) to fail fast, improving correctness of config-driven workflows.
Changes:
- Implement two-pass template execution: detect missing keys with
missingkey=error, then warn and fall back tomissingkey=zero(or hard error in strict mode). - Add
strict_templatespipeline config and propagate it through engine → pipeline → pipeline context. - Update tests and documentation; includes a number of gofmt/alignment-only changes across unrelated files.
Reviewed changes
Copilot reviewed 7 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/step_schema_builtins.go | Formatting-only alignment in built-in step schema registration. |
| schema/coercion.go | Formatting-only alignment in type coercion registry map. |
| plugin/autofetch_test.go | Formatting-only alignment in test table fields/comments. |
| module/step_output_redactor_test.go | Formatting-only alignment in test table struct fields. |
| module/pipeline_template_test.go | Add tests for missing-key WARN logging and strict mode error behavior. |
| module/pipeline_template.go | Implement strict/non-strict missing-key handling with warning + fallback execution. |
| module/pipeline_step_deploy_rolling_test.go | Formatting-only alignment in test config map. |
| module/pipeline_step_deploy_rolling.go | Formatting-only alignment in HTTPDeployDriver struct fields. |
| module/pipeline_step_deploy_rollback_test.go | Formatting-only alignment in test config map. |
| module/pipeline_step_deploy_rollback.go | Formatting-only alignment in structs/output map. |
| module/pipeline_step_deploy_canary_test.go | Formatting-only alignment in mock driver struct fields. |
| module/pipeline_step_deploy_blue_green_test.go | Formatting-only alignment in mock driver struct fields. |
| module/pipeline_step_container_build_test.go | Formatting-only alignment in mock struct fields. |
| module/pipeline_step_ai_extract_test.go | Formatting-only alignment in test config map. |
| module/pipeline_step_ai_classify_test.go | Formatting-only alignment in test config map. |
| module/pipeline_executor.go | Add StrictTemplates to Pipeline and propagate StrictTemplates/logger into PipelineContext. |
| module/infra_module_test.go | Formatting-only alignment in mock app methods and test config map. |
| module/infra_module_deploy_bridge_test.go | Formatting-only alignment in mock methods/fields. |
| module/iac_state_gcs_test.go | Formatting-only alignment in mock struct fields/comments. |
| module/iac_state_azure_test.go | Formatting-only alignment in mock struct fields/comments. |
| interfaces/pipeline.go | Add StrictTemplates and Logger fields to PipelineContext. |
| interfaces/iac_test.go | Formatting-only alignment in mock methods. |
| engine.go | Wire pipeCfg.StrictTemplates into constructed pipelines. |
| config/pipeline.go | Add strict_templates field to PipelineConfig. |
| cmd/wfctl/test.go | Formatting-only alignment in CLI test struct fields. |
| cmd/wfctl/plugin_install_new_test.go | Formatting-only alignment in test tarball entry maps. |
| cmd/wfctl/main.go | Formatting-only alignment in commands map. |
| cmd/wfctl/infra_state.go | Formatting-only alignment in structs used for state import/export. |
| DOCUMENTATION.md | Document missing-key behavior, WARN default, and strict opt-in. |
Comment on lines
+440
to
+459
| To turn the warning into a hard error, set `strict_templates: true` on the pipeline: | ||
|
|
||
| ```yaml | ||
| pipelines: | ||
| my-pipeline: | ||
| strict_templates: true # any missing key access fails the pipeline step | ||
| steps: | ||
| - name: process | ||
| type: step.set | ||
| config: | ||
| values: | ||
| tenant: "{{ .steps.auth.affilate_id }}" # typo: affilate_id instead of affiliate_id → step fails immediately | ||
| ``` | ||
|
|
||
| | Mode | `strict_templates` | Missing key result | | ||
| |------|-------------------|--------------------| | ||
| | Default | `false` | Zero value (`<no value>`) + WARN log | | ||
| | Strict | `true` | Step returns an error | | ||
|
|
||
| `wfctl template validate --config workflow.yaml` lints template expressions and warns on undefined step references and forward references. Use `strict_templates: true` in the pipeline config to catch field-level typos at runtime. |
There was a problem hiding this comment.
The docs imply strict_templates: true turns missing-key access into a hard error generally, but as implemented the step/trigger template helper functions still return nil on missing keys (so strict mode won't catch typos when using the documented "Preferred syntax"). Either adjust the implementation to make those helpers error in strict mode, or update this section to spell out the current limitation so users aren't misled.
module/pipeline_template.go
Outdated
Comment on lines
+398
to
+404
| // Log a warning about the missing key so developers can spot typos. | ||
| logger := slog.Default() | ||
| if pc != nil && pc.Logger != nil { | ||
| logger = pc.Logger | ||
| } | ||
| logger.Warn("template resolved missing key to zero value", | ||
| "template", tmplStr, |
There was a problem hiding this comment.
The WARN log includes the full (preprocessed) template string. Pipeline templates can contain literals (including secrets/PII) and logging them on missing-key access can leak sensitive data into logs. Consider omitting the full template text (or truncating/sanitizing it) and instead logging safer context (e.g., pipeline name from pc.Metadata["pipeline"], plus the missing-key error) so the warning is still actionable without exposing configuration contents.
Suggested change
| // Log a warning about the missing key so developers can spot typos. | |
| logger := slog.Default() | |
| if pc != nil && pc.Logger != nil { | |
| logger = pc.Logger | |
| } | |
| logger.Warn("template resolved missing key to zero value", | |
| "template", tmplStr, | |
| // Log a warning about the missing key so developers can spot typos, | |
| // without including the full template text (which may contain secrets). | |
| logger := slog.Default() | |
| if pc != nil && pc.Logger != nil { | |
| logger = pc.Logger | |
| } | |
| pipelineName := "<unknown>" | |
| if pc != nil && pc.Metadata != nil { | |
| if v, ok := pc.Metadata["pipeline"]; ok { | |
| pipelineName = fmt.Sprint(v) | |
| } | |
| } | |
| logger.Warn("template resolved missing key to zero value", | |
| "pipeline", pipelineName, |
module/pipeline_template.go
Outdated
Comment on lines
+358
to
+364
| // Missing key behaviour: | ||
| // - When pc.StrictTemplates is true (Option A), any reference to a missing | ||
| // map key causes an immediate error, surfacing typos as failures. | ||
| // - When pc.StrictTemplates is false (the default, Option C), a missing key | ||
| // resolves to the zero value AND a WARN log is emitted via pc.Logger (or | ||
| // slog.Default() when no logger is set) so that the silent failure is | ||
| // visible without breaking existing pipelines. |
There was a problem hiding this comment.
Strict mode currently only affects Go template map key access via missingkey=error. The context helper functions step and trigger still return nil on missing keys, so strict_templates: true will not fail for typos when templates use these helpers (which are documented as the preferred syntax for hyphenated step names). Either update step/trigger to return an error in strict mode (template funcs can return (any, error)) and add tests for that behavior, or clarify in the docs/comments that strict mode does not apply to these helpers.
Suggested change
| // Missing key behaviour: | |
| // - When pc.StrictTemplates is true (Option A), any reference to a missing | |
| // map key causes an immediate error, surfacing typos as failures. | |
| // - When pc.StrictTemplates is false (the default, Option C), a missing key | |
| // resolves to the zero value AND a WARN log is emitted via pc.Logger (or | |
| // slog.Default() when no logger is set) so that the silent failure is | |
| // visible without breaking existing pipelines. | |
| // Missing key behaviour (direct map access): | |
| // - When pc.StrictTemplates is true (Option A), any reference to a missing | |
| // map key (e.g. .Steps.foo.bar) causes an immediate error via | |
| // missingkey=error, surfacing typos as failures. | |
| // - When pc.StrictTemplates is false (the default, Option C), a missing key | |
| // resolves to the zero value AND a WARN log is emitted via pc.Logger (or | |
| // slog.Default() when no logger is set) so that the silent failure is | |
| // visible without breaking existing pipelines. | |
| // | |
| // NOTE: Strict template mode only affects direct map key resolution driven by | |
| // missingkey=error. It does NOT change the behaviour of helper functions such | |
| // as step or trigger, which may still return nil for unknown keys; typos when | |
| // using those helpers will therefore not cause Resolve to fail even when | |
| // pc.StrictTemplates is true. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Contributor
|
@copilot apply changes based on the comments in this thread |
…om warn log Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/cf41e59f-d4e3-409b-9717-d16b8e2d68b4
Contributor
Author
Applied all three review changes in d42e671:
|
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.
missingkey=zerosilently resolved typos in step field references (e.g..steps.auth.affilate_id) to empty string, producing invisibly wrong data with no error or log signal at any stage.Changes
Option C — runtime warning (default, backward-compatible)
TemplateEngine.Resolve()now attempts execution withmissingkey=errorfirst; on a missing-key hit, logs aWARNvia the pipeline's logger (falling back toslog.Default()) then retries withmissingkey=zeropc.Metadata["pipeline"]) instead of the full template string to avoid leaking secrets/PII into logsOption A — strict mode (opt-in per pipeline)
strict_templates: truepipeline config field; when set, missing key access immediately fails the step instead of warning+continuing{{ .steps.auth.field }}) and thestep/triggerhelper functions ({{ step "auth" "field" }})Without
strict_templates, the same typo produces:Key files
interfaces/pipeline.go—PipelineContextgainsStrictTemplates bool+Logger *slog.Loggerconfig/pipeline.go—PipelineConfiggainsstrict_templatesfieldmodule/pipeline_executor.go—PipelinegainsStrictTemplates; propagates both to context inExecute()module/pipeline_template.go— rewritesResolve()with two-pass detection +isMissingKeyError()helper;step/triggerhelpers return(any, error)and propagate errors in strict modeengine.go— readspipeCfg.StrictTemplateswhen constructing pipelinesDOCUMENTATION.md— "Missing Key Behaviour" section added under template docsOriginal prompt
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.