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: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

No I think we need to understand the problem first.
We break workflows because now we can never have "" as a valid parameter if we accept your original change.

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

KowalczykBartek · 2026-05-11T11:31:28Z

@isubasinghe should I pull recent changes and rebase ?

KowalczykBartek · 2026-05-14T18:53:21Z

@isubasinghe any update ?

isubasinghe · 2026-05-18T05:47:13Z

I responded in the thread just now. I think we need to fix the issue I raised.

…ch-signed-verified

isubasinghe · 2026-05-19T06:21:44Z

Also @KowalczykBartek do you want me to have a look and investigate as to why the above issue happens?

Or do you want me to take over the PR?
I am happy to guide you to get this PR across the finish line

KowalczykBartek · 2026-05-19T06:38:10Z

@isubasinghe - i will try - not sure i can understand wider perspective - give me couple of days

isubasinghe · 2026-05-19T06:45:28Z

No worries, feel free to ask me questions if you would like as well. I am on the CNCF slack.
The argo workflows codebase is not easy to navigate.

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

Comment thread workflow/controller/steps.go Outdated

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>

Merge branch 'main' into fix-unresolved-reference-for-child-main-bran…

ad343c6

…ch-signed-verified

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!

Uh oh!

Uh oh!

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!

isubasinghe May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KowalczykBartek commented May 11, 2026

Uh oh!

KowalczykBartek commented May 14, 2026

Uh oh!

isubasinghe commented May 18, 2026

Uh oh!

isubasinghe commented May 19, 2026

Uh oh!

KowalczykBartek commented May 19, 2026

Uh oh!

isubasinghe commented May 19, 2026

KowalczykBartek commented Apr 15, 2026 •

edited

Loading

KowalczykBartek commented Apr 26, 2026 •

edited

Loading

isubasinghe May 18, 2026 •

edited

Loading