fix: populate scope with empty values for outputs of skipped/omitted … by KowalczykBartek · Pull Request #15932 · argoproj/argo-workflows

KowalczykBartek · 2026-04-15T06:55:22Z

Fixes #15873
Motivation

Try to resolve child's output when steps is actually skipped.

utafrali

The core logic is sound: Template.Outputs is a value type so there is no nil dereference risk, and the empty-string scope entries correctly unblock downstream when expressions that reference skipped step outputs. The main gaps are missing artifact output handling (a real failure mode for artifact references), shadowed variable names, and a silently dropped error from ResolveTemplate.

utafrali · 2026-04-15T07:02:24Z

 				woc.buildLocalScope(stepsCtx.scope, prefix, childNode)
+
+				if (childNode.Phase == wfv1.NodeSkipped || childNode.Phase == wfv1.NodeOmitted) && childNode.Outputs == nil {
+					_, tmpl, _, err := stepsCtx.tmplCtx.ResolveTemplate(ctx, &step)


:= here shadows two outer-scoped names: the tmpl parameter of executeSteps and the loop-level err. While the inner block scope keeps this from being a correctness bug, shadow linters (and code readers) will flag it. Use distinct names:

stepTmpl, resolveErr := stepsCtx.tmplCtx.ResolveTemplate(ctx, &step) if resolveErr == nil && stepTmpl != nil {

utafrali · 2026-04-15T07:02:24Z

+
+				if (childNode.Phase == wfv1.NodeSkipped || childNode.Phase == wfv1.NodeOmitted) && childNode.Outputs == nil {
+					_, tmpl, _, err := stepsCtx.tmplCtx.ResolveTemplate(ctx, &step)
+					if err == nil && tmpl != nil {


When ResolveTemplate fails the error is silently dropped and the scope remains unpopulated, so a downstream step will still fail with an unresolvable reference. The caller gets no indication that template resolution was attempted and failed. A debug log here would make production troubleshooting much easier:

if resolveErr != nil { woc.log.WithError(resolveErr).Debug(ctx, "failed to resolve template for skipped step, outputs will not be populated in scope") }

utafrali · 2026-04-15T07:02:24Z

+						}
+						if tmpl.Outputs.Result != nil {
+							stepsCtx.scope.addParamToScope(fmt.Sprintf("%s.outputs.result", prefix), "")
+						}


The fix handles Parameters and Result but skips Artifacts. If any downstream step (or when expression) references steps.<skipped>.outputs.artifacts.<name>, it will still fail with an unresolvable reference. addArtifactToScope accepts a wfv1.Artifact value, so you can register a zero-value placeholder:

for _, artifact := range tmpl.Outputs.Artifacts { key := fmt.Sprintf("%s.outputs.artifacts.%s", prefix, artifact.Name) stepsCtx.scope.addArtifactToScope(key, wfv1.Artifact{}) }

This mirrors how the parameter case is handled above.

utafrali · 2026-04-15T07:02:24Z

 		})
 }

+func (s *FunctionalSuite) TestStepsSkippedOutputRef() {


The test exercises NodeSkipped but never NodeOmitted. Both phases are handled in the fix. Since Omitted is a distinct node phase that arises in DAG templates (not steps), it may require a separate workflow fixture, but at least a comment explaining why it isn't tested would clarify the intent.

Hi, sorry for that, but honestly its my first time I wrote golang and did something in argo -

why it isn't tested would clarify the intent.

honestly - I just fixed the case which blocks us, that's the reason, I can try to fix other cases as well but this can take some time.

If its enough to write comment, can you please sketch some comment I can paste ? I will probably not create anything meaningful :/

@utafrali I pushed following comment on that test

// TestStepsSkippedOutputRef tests that output references to skipped steps resolve to empty strings // rather than causing an unresolvable reference error. NodeOmitted is not exercised here because // that phase arises in DAG templates, not steps; the fix covers both phases in the same code path.

KowalczykBartek · 2026-04-15T07:46:52Z

@utafrali thank you for review 🙇 - I applied what I could and tested it, I have question only regarding the comment for test.

KowalczykBartek · 2026-04-21T08:27:14Z

@isubasinghe is there a chance to push on that ?

isubasinghe · 2026-04-26T06:18:02Z

Hey could you please lint and rebase this? I really want to get this merged.

KowalczykBartek · 2026-04-26T07:23:35Z

@isubasinghe lint ?

isubasinghe · 2026-04-26T07:33:32Z

Yes please run make lint.

See https://github.com/argoproj/argo-workflows/actions/runs/24570724852/job/71842877000?pr=15932

…steps (argoproj#15873) Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

KowalczykBartek · 2026-04-26T08:02:41Z

I have no idea what I just did - @isubasinghe pls check if now its ok

edit: lint is green now

isubasinghe · 2026-04-28T02:56:40Z

Hmm an end to end test is broken according to the logs.

KowalczykBartek · 2026-04-28T06:52:22Z

@isubasinghe i wrote one test it pass

Condition "to be done" met after 19s
Checking expectation steps-skipped-output-ref-mt7t9
steps-skipped-output-ref-mt7t9 : Succeeded 
=== PASS: FunctionalSuite/TestStepsSkippedOutputRef
=== SLOW TEST:  FunctionalSuite/TestStepsSkippedOutputRef took 19s
--- PASS: TestFunctionalSuite (19.55s)
    --- PASS: TestFunctionalSuite/TestStepsSkippedOutputRef (19.53s)
PASS
ok  	github.com/argoproj/argo-workflows/v4/test/e2e	20.288s

not sure what i can do

isubasinghe · 2026-04-28T06:55:39Z

https://github.com/argoproj/argo-workflows/actions/runs/24951771817/job/73307331648?pr=15932

This is failing though, please do check the Github checks.

Are you interacting with this via CLI or not via the Github UI?

Can you see this bit?

KowalczykBartek · 2026-04-28T06:59:42Z

ok now I see :/ let me check

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

KowalczykBartek · 2026-04-28T08:13:19Z

@isubasinghe do I see correctly it succeeded now or it need to be forced to run ?

KowalczykBartek · 2026-04-30T22:26:00Z

@isubasinghe any update ?

isubasinghe · 2026-05-04T02:39:36Z

 			} else {
 				woc.buildLocalScope(stepsCtx.scope, prefix, childNode)
+
+				if (childNode.Phase == wfv1.NodeSkipped || childNode.Phase == wfv1.NodeOmitted) && childNode.Outputs == nil {


I don't quite understand why there is this nil check here.
If a node has been set to skipped or omitted, the outputs should be nil, this not being the case is arguably a mistake?

I believe the default param population happens in line 198.

Oh no I was just asking you why you added this check.

why you added this check.

honestly - i don't know :) I just wanted to craft something asap, i have little domain knowledge around that codebase, but you are probably right, if node is skipped or omitted it doesn't matter what is the output we will override it.

isubasinghe · 2026-05-04T02:41:43Z

+						woc.log.WithError(resolveErr).Debug(ctx, "failed to resolve template for skipped step, outputs will not be populated in scope")
+					}
+
+					if resolveErr == nil && tmpl != nil {


Did you mean to use stepTmpl here?

isubasinghe · 2026-05-04T02:42:02Z

+							stepsCtx.scope.addParamToScope(key, "")
+						}
+						// register a zero-value placeholder for artifacts
+						for _, artifact := range tmpl.Outputs.Artifacts {


Same here, did you mean to use stepTmpl?

isubasinghe · 2026-05-04T02:59:54Z

+			} else if val == "" && param.ValueFrom.Default != nil {
+				// A skipped/omitted step contributes "" to scope; treat that as missing and use the default
+				val = param.ValueFrom.Default.String()
 			}


Revert this change, val could validly be "", we shouldn't override it in that case.

@isubasinghe after this change tests fails

Okay I think this means we need a way to determine "" from this yielded "" and this was omitted/skipped.

so I reversed it - is that ok ?

cc @isubasinghe

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

KowalczykBartek mentioned this pull request Apr 15, 2026

fix: populate scope with empty values for outputs of skipped/omitted … #15926

Closed

utafrali reviewed Apr 15, 2026

View reviewed changes

KowalczykBartek mentioned this pull request Apr 21, 2026

Output from a Skipped Step Used as a Parameter Causes error=requeue and Workflow hangs #15949

Open

4 tasks

KowalczykBartek added 4 commits April 26, 2026 10:00

fix: populate scope with empty values for outputs of skipped/omitted …

cae806c

…steps (argoproj#15873) Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

fix: apply comments from review (argoproj#15873)

399a91f

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

fix: add comment to test case (argoproj#15873)

5cc2576

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

fix: run make lint (argoproj#15873)

421e737

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

KowalczykBartek force-pushed the fix-unresolved-reference-for-child-main-branch-signed-verified branch from 0ae31ea to 421e737 Compare April 26, 2026 08:01

fix: fix broken test case (argoproj#15873)

b2dd26f

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

isubasinghe reviewed May 4, 2026

View reviewed changes

isubasinghe requested changes May 4, 2026

View reviewed changes

fix: apply review comments (argoproj#15873)

8395ecd

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

KowalczykBartek requested a review from isubasinghe May 4, 2026 14:45

fix: apply review comments (argoproj#15873)

7ad6ea6

Signed-off-by: KowalczykBartek <bkowalczyyk@gmail.com>

Conversation

KowalczykBartek commented Apr 15, 2026

Uh oh!

utafrali left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KowalczykBartek commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KowalczykBartek commented Apr 21, 2026

Uh oh!

isubasinghe commented Apr 26, 2026

Uh oh!

KowalczykBartek commented Apr 26, 2026

Uh oh!

isubasinghe commented Apr 26, 2026

Uh oh!

KowalczykBartek commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isubasinghe commented Apr 28, 2026

Uh oh!

KowalczykBartek commented Apr 28, 2026

Uh oh!

isubasinghe commented Apr 28, 2026

Uh oh!

KowalczykBartek commented Apr 28, 2026

Uh oh!

KowalczykBartek commented Apr 28, 2026

Uh oh!

KowalczykBartek commented Apr 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

KowalczykBartek commented Apr 15, 2026 •

edited

Loading

KowalczykBartek commented Apr 26, 2026 •

edited

Loading