fix: use input.defaults for suspend templates

[Joibel]

Motivation

This workflow apparently worked in 3.4, I haven't tried to work out why.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: suspend-with-input-defaults
spec:
  entrypoint: main
  templates:
  - name: main
    steps:
    - - name: wait
        template: wait
  - name: wait
    suspend:
      duration: "0s"
    inputs:
      parameters:
        - name: input
          default: "Hello World"
        - name: select
          default: default
          enum:
            - default
            - option1
            - option2
    outputs:
      parameters:
        - name: input
          valueFrom:
            supplied: {}
        - name: select
          valueFrom:
            supplied: {}

Modifications

We now observe the default values of the inputs if the output doesn't have defaults, for suspend templates only

Verification

New unit test, which verified the workflow passes and checks the values.
The workflow would formerly failed.

Documentation

None required, bug fix

Summary by CodeRabbit

Release Notes

• New Features
  • Suspend nodes can now inherit default values from their corresponding input parameters when output parameters lack a specific default, enabling workflows to resolve gracefully when nodes timeout without explicit output defaults.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[Joibel]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add a fallback mechanism to suspend node handling in the workflow operator. When an output parameter lacks an explicit default but was supplied, the operator now searches the corresponding input parameters for a matching name with a default value and uses that. This enables suspend nodes to inherit defaults from their inputs when no specific output default is provided.

Changes

Cohort / File(s)	Summary
Core Functionality workflow/controller/operator.go	Adds fallback logic to propagate default values from input parameters to output parameters in suspend nodes when the output parameter has a ValueFrom.Supplied but no explicit Default. Enables output parameters to resolve with input defaults during suspend node timeout scenarios.
Test Coverage workflow/controller/operator_test.go	Introduces test fixture suspendWithInputDefaultsTemplate (YAML workflow with suspend node inputs/outputs) and test TestSuspendTimeoutWithInputDefaults to verify that suspend node outputs inherit input parameter defaults when a timeout occurs, allowing the workflow to proceed with correct default values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description addresses motivation and verification but lacks explicit coverage of the issue number and formal structure required by the template.	Add 'Fixes #12896' to the description to link the issue automatically, and ensure all template sections (Fixes, Motivation, Modifications, Verification, Documentation) are explicitly formatted as headers.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: use input.defaults for suspend templates' is concise, directly references the main change (using input defaults for suspend templates), and accurately reflects the primary objective of fixing issue #12896.
Linked Issues check	✅ Passed	The code changes fully address issue #12896 by implementing fallback logic to assign input parameter defaults to output parameters when outputs use valueFrom.supplied without their own defaults, resolving the issue where enum inputs and outputs ignored default values for suspend templates.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the suspend template default values issue: operator.go adds the default fallback logic, and operator_test.go adds a test specifically validating the fix for suspend timeout with input defaults.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @workflow/controller/operator_test.go:
- Around line 2999-3049: In suspendWithInputDefaultsTemplate update the select
parameter default to be quoted (change default: default to default: "default")
to keep scalar style consistent, and clarify the suspend.duration: "0s" behavior
by either adding a brief inline comment in the template next to suspend.duration
explaining it triggers immediate auto-resume or renaming the test
(suspend-with-input-defaults) to indicate it tests auto-resume/timeout behavior;
target the suspendWithInputDefaultsTemplate string and the select
parameter/default and suspend.duration entries when making the change.

🧹 Nitpick comments (1)

workflow/controller/operator.go (1)

3855-3866: Consider using DeepCopy for defensive pointer handling.

The logic correctly addresses the issue where suspend node outputs should inherit input defaults. However, the direct pointer assignment param.ValueFrom.Default = inParam.Default causes both the input and output parameters to share the same underlying *AnyString object.

While this is likely safe in practice (defaults are typically not mutated after initialization), using a deep copy would be more defensive and prevent potential unintended side effects if either value were ever modified elsewhere.

🛠️ Suggested defensive copy

 			if param.ValueFrom.Default == nil {
 				for _, inParam := range tmpl.Inputs.Parameters {
 					if inParam.Name == param.Name && inParam.Default != nil {
-						param.ValueFrom.Default = inParam.Default
+						param.ValueFrom.Default = inParam.Default.DeepCopy()
 						break
 					}
 				}
 			}

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4540e18 and f1616c6.

📒 Files selected for processing (2)

• workflow/controller/operator.go
• workflow/controller/operator_test.go

🧰 Additional context used

🧬 Code graph analysis (2)

workflow/controller/operator_test.go (2)

pkg/apis/workflow/v1alpha1/workflow_types.go (2)

• Outputs (1623-1639)
• Parameter (1000-1025)

pkg/apis/workflow/v1alpha1/marshall.go (1)

• MustUnmarshalWorkflow (71-75)

workflow/controller/operator.go (2)

pkg/apis/workflow/v1alpha1/workflow_types.go (2)

• ValueFrom (1028-1056)
• Inputs (975-987)

workflow/common/params.go (1)

• Parameters (6-6)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Lint
• GitHub Check: Build Binaries (cli)
• GitHub Check: Build Binaries (controller)
• GitHub Check: Windows Unit Tests
• GitHub Check: Unit Tests
• GitHub Check: argo-images (argocli)
• GitHub Check: argo-images (argoexec)

[eduardodbr]

This PR fixes the output issue but it doesn't address the UI issue mentioned in #12896 .

I did take a look at the UI code, and I believe the issue is here:

In ui/src/workflows/components/workflow-details/workflow-details.tsx:
https://github.com/argoproj/argo-workflows/blob/e95af2a160aa2d9d4246afacebf8272b8d45dd1e/ui/src/workflows/components/workflow-details/workflow-details.tsx#L137C4-L147C11

- if (paramClone.enum) {
- paramClone.value = paramClone.default;
- }
+ if (paramClone.enum && !paramClone.value) {
+ paramClone.value = paramClone.default;
+ }

Currently, it overwrites paramClone.value with paramClone.default whenever there's an enum. But this is wrong because The parameter might already have a value set (from arguments) -> this is the case shown in #12896

[Joibel]

This PR fixes the output issue but it doesn't address the UI issue mentioned in #12896 .

I did take a look at the UI code, and I believe the issue is here:

In ui/src/workflows/components/workflow-details/workflow-details.tsx: https://github.com/argoproj/argo-workflows/blob/e95af2a160aa2d9d4246afacebf8272b8d45dd1e/ui/src/workflows/components/workflow-details/workflow-details.tsx#L137C4-L147C11

- if (paramClone.enum) {
- paramClone.value = paramClone.default;
- }
+ if (paramClone.enum && !paramClone.value) {
+ paramClone.value = paramClone.default;
+ }

Currently, it overwrites paramClone.value with paramClone.default whenever there's an enum. But this is wrong because The parameter might already have a value set (from arguments) -> this is the case shown in #12896

I'd prefer to fix the UI issue separately as it's not the same bug. I've removed the "Fixes #12896" from the desctiption.

[eduardodbr]

I can submit a fix to the UI then, since I already looked into it

[argo-cd-cherry-pick-bot]

🍒 Cherry-pick PR created for 4.0: #15482

[argo-cd-cherry-pick-bot]

🍒 Cherry-pick PR created for 3.7: #15483

[argo-cd-cherry-pick-bot]

🍒 Cherry-pick PR created for 3.6: #15484