Skip to content

feat(workflows): interactive steps run in CI via non-TTY defaults - #2709

Closed
Brian Ojeda (sgtoj) wants to merge 3 commits into
cloudposse:mainfrom
sgtoj:feat/interactive-step-ci-defaults
Closed

Brian Ojeda (sgtoj) wants to merge 3 commits into
cloudposse:mainfrom
sgtoj:feat/interactive-step-ci-defaults

Conversation

@sgtoj

@sgtoj Brian Ojeda (sgtoj) commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Interactive workflow and custom-command step types (choose, input, confirm, filter, file, write) now fall back to their configured default value when running without a TTY (e.g. in CI) instead of failing with interactive terminal required for step. Without a default, the historical TTY-required error is preserved, so an unattended run never proceeds with an unintended value.

This lands as three cohesive parts:

  1. Non-TTY default fallback (pkg/runner/step) — a shared BaseHandler.resolveInteractive helper drives the decision for every interactive handler. Because workflows, custom commands, hooks, and the task runner all execute steps through the same handler registry, one change covers them all.
  2. YAML functions in workflow step fields (internal/exec) — workflow step default/prompt/options/placeholder now evaluate the context-free !env and !exec functions. Workflow manifests previously left these as literal !env ... strings; custom commands defined in atmos.yaml already resolved them. Stack-dependent functions (!terraform.output, !store, ...) are intentionally not evaluated here (a workflow step has no component/stack context).
  3. Step-variable templating in workflow commands (internal/exec) — workflow shell/atmos/exec step commands resolve {{ .steps.* }}, {{ .env.* }}, and {{ .flags.* }}, so values captured by an interactive step (or its CI default) flow into later steps — parity with custom command steps.

Example that now runs both locally (prompts) and in CI (uses defaults):

workflows:
  build-manifests:
    steps:
      - name: account
        type: choose
        prompt: "Account"
        options: [dev, prod]
        default: !env STACK_ACCOUNT dev
      - name: tag
        type: input
        prompt: "Release tag"
        default: !env RELEASE_TAG latest
      - type: shell
        command: atmos kube build "{{ .steps.account.value }}/tao" --tag "{{ .steps.tag.value }}"

Why

Interactive steps made workflows and custom commands friendly to run by hand but unrunnable in CI, forcing teams to maintain a second, prompt-free copy of the same automation. A configured default is an explicit opt-in to a non-interactive value, so honoring it when there is no TTY is safe and removes the need for duplicate definitions. Leaving default unset keeps the guard for steps where a human must choose.

Testing

  • Unit tests for each interactive handler: single/multi selection, confirm parsing, template-resolved defaults, and ErrStepTTYRequired when no default is set.
  • Unit tests for !env/!exec resolution (and passthrough of plain, template, and stack-dependent values), nested steps, and {{ .steps.* }} / {{ .env.* }} command resolution.
  • Validated end-to-end with a built binary for both a workflow and a custom command, with and without environment overrides.

References

  • PRD: docs/prd/interactive-steps-non-tty-defaults.md
  • Changelog: website/blog/2026-07-07-interactive-steps-ci-defaults.mdx
  • Roadmap: Workflows Overhaul milestone in website/src/data/roadmap.js

Summary by CodeRabbit

  • New Features
    • Interactive steps now support non-TTY execution by using a configured default value instead of prompting.
    • Step values can now be reused in later commands, and defaults can be sourced dynamically from environment-driven expressions.
  • Bug Fixes
    • Preserved the existing error when an interactive step has no default and no terminal is available.
    • Improved handling for multi-select defaults and stored values across nested workflow steps.
  • Documentation
    • Expanded workflow and command docs with CI/non-TTY behavior examples and usage guidance.

Interactive step types (choose, input, confirm, filter, file, write) now
use their configured `default` value when running without a TTY (e.g. in
CI) instead of failing with ErrStepTTYRequired. Without a default, the
historical TTY-required error is preserved.

Also evaluate value-producing YAML functions (!env, !exec) in workflow
interactive step fields (default/prompt/placeholder/options) so defaults
can be sourced from the environment, and resolve step-variable templates
({{ .steps.* }}, {{ .env.* }}, {{ .flags.* }}) in workflow shell/atmos/exec
step commands (parity with custom command steps).

Covers workflows and custom commands via the shared step handler registry.
- Document non-TTY default fallback in the steps overview and the six
  interactive step type pages (choose, input, confirm, filter, file, write),
  adding the `default` field to filter/file/write.
- Add a 'Running Interactive Steps in CI (non-TTY)' section to custom command
  steps docs.
- Add changelog blog post and a Workflows Overhaul roadmap milestone.
- Add PRD docs/prd/interactive-steps-non-tty-defaults.md.
@sgtoj
Brian Ojeda (sgtoj) requested a review from a team as a code owner July 9, 2026 13:58
@atmos-pro

atmos-pro Bot commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

@github-actions github-actions Bot added the size/l Large size PR label Jul 9, 2026
@sgtoj
Brian Ojeda (sgtoj) marked this pull request as draft July 9, 2026 14:11
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a shared non-TTY interactive fallback (resolveInteractive) to BaseHandler, used by choose, confirm, input, file, filter, and write handlers to return configured defaults without prompting when no TTY is present. Adds workflow-level !env/!exec YAML function resolution for interactive step fields and command templating via resolveWorkflowStepCommand, wired into ExecuteWorkflow. Includes unit tests, docs, blog post, and roadmap entry.

Changes

Non-TTY interactive defaults and workflow templating

Layer / File(s) Summary
Design document
docs/prd/interactive-steps-non-tty-defaults.md
PRD describing non-TTY default fallback, YAML function resolution, and command templating design.
Shared resolveInteractive logic
pkg/runner/step/handler_base.go
Adds hasInteractiveTTY, ttyRequiredError, and resolveInteractive to decide prompt vs. default fallback.
Interactive handlers adopt resolveInteractive
pkg/runner/step/choose.go, confirm.go, input.go, file.go, filter.go, write.go
Handlers replace CheckTTY with resolveInteractive, returning resolved defaults (including multi-value filter split and confirm boolean parsing) in non-TTY mode.
Handler tests
pkg/runner/step/interactive_default_test.go
Tests cover resolveInteractive decisions, default fallback, missing-default errors, and default templating.
Workflow YAML function resolution
internal/exec/workflow_step_functions.go
Adds resolveWorkflowStepFunctions and helpers to evaluate !env/!exec in interactive step fields, recursing into nested steps.
Workflow command templating
internal/exec/workflow_step_functions.go
Adds workflowCommandSupportsTemplating and resolveWorkflowStepCommand to resolve {{ .steps.* }}, {{ .env.* }}, {{ .flags.* }} in commands.
ExecuteWorkflow wiring
internal/exec/workflow_utils.go
Calls function/command resolution during workflow execution, wrapping failures as ErrWorkflowStepFailed.
Workflow tests
internal/exec/workflow_step_functions_test.go
Tests for function detection, resolution, nested steps, nil definitions, templating support, and command resolution.
Documentation and blog post
website/docs/..., website/blog/*, website/src/data/roadmap.js
Docs and blog post explain non-TTY defaults, !env/!exec support, and template variable propagation; adds roadmap entry and blog author.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related PRs

  • cloudposse/atmos#1229: Both modify internal/exec/workflow_utils.go's workflow step failure/error handling, which this PR's templating errors build on.
  • cloudposse/atmos#1899: This PR's resolveInteractive and handler Execute changes build on that PR's registry-based interactive handlers and BaseHandler TTY enforcement.

Suggested labels: minor

Suggested reviewers: aknysh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: interactive workflow steps now use non-TTY defaults in CI.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
pkg/runner/step/choose.go (1)

94-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Same resolveDefault body duplicated across four handlers — hoist into BaseHandler.

choose.go, file.go, filter.go, and write.go each define an identical private resolveDefault (empty-check → vars.Resolve → wrap error). BaseHandler.ResolvePrompt already does the same thing for the Prompt field — mirror it for Default and delete the four copies. This would also make confirm.go's missing-templating bug (see comment there) structurally impossible to reintroduce.

♻️ Suggested consolidation
+// ResolveDefault resolves template variables in the step's default value.
+func (h BaseHandler) ResolveDefault(step *schema.WorkflowStep, vars *Variables) (string, error) {
+	defer perf.Track(nil, "step.BaseHandler.ResolveDefault")()
+
+	if step.Default == "" {
+		return "", nil
+	}
+	resolved, err := vars.Resolve(step.Default)
+	if err != nil {
+		return "", errUtils.Build(errUtils.ErrTemplateEvaluation).
+			WithCause(err).
+			WithContext("step", step.Name).
+			WithContext("field", "default").
+			Err()
+	}
+	return resolved, nil
+}

Then replace each handler's local resolveDefault with a call to h.ResolveDefault(step, vars).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/runner/step/choose.go` around lines 94 - 103, The same resolveDefault
logic is duplicated in ChooseHandler, FileHandler, FilterHandler, and
WriteHandler, so move it into BaseHandler alongside ResolvePrompt and make it
handle the Default field with the same empty-check, vars.Resolve call, and
wrapped error. Update the handler methods in choose.go, file.go, filter.go, and
write.go to call h.ResolveDefault(step, vars) instead of keeping private copies,
and remove the duplicated implementations.
pkg/runner/step/interactive_default_test.go (1)

168-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Templating test only covers choose — extend it table-driven across handlers.

This is the only test asserting default templating ({{ .env.* }}), and it exercises just the choose type. It's a single-case test today, while the sibling TestInteractiveHandlers_NonTTYUsesDefault is properly table-driven. Broadening this into a table-driven test across input, confirm, file, filter, write would have caught the confirm.go templating bug flagged in confirm.go.

As per coding guidelines, "Use table-driven tests for testing multiple scenarios in Go."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/runner/step/interactive_default_test.go` around lines 168 - 194, The
non-TTY default-templating coverage is too narrow because
TestInteractiveHandlers_NonTTYDefaultTemplating only exercises the choose
handler. Refactor this test into a table-driven test like
TestInteractiveHandlers_NonTTYUsesDefault and run the same {{ .env.* }}
default-templating assertion across the relevant interactive handlers (input,
confirm, file, filter, write, and choose). Use Get("...") with each handler name
and verify Execute returns the rendered default value for each case so
templating regressions in handlers like confirm are covered.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/exec/workflow_step_functions_test.go`:
- Around line 70-73: The test in resolveStepFunctionString should assert the
specific sentinel error instead of just any error. Update the “malformed env
function returns an error” case to use require.ErrorIs against
pkg/utils.ErrInvalidAtmosYAMLFunction, keeping the existing
resolveStepFunctionString and atmosConfig setup intact.

In `@internal/exec/workflow_step_functions.go`:
- Around line 146-155: The current env handling in workflowStepFunction reuses
stepExecutorState.Variables(), so SetEnv mutates shared state and can leak
values into later steps. Update the logic in workflowStepFunction to scope env
overrides only to the current command rendering, either by working on a
step-local copy of the Variables state before calling Resolve or by restoring
the original env map immediately after Resolve completes. Keep the fix centered
around the workflowStepFunction and its Variables/SetEnv/Resolve flow so later
steps do not see prior step env values.

In `@pkg/runner/step/confirm.go`:
- Around line 41-55: The ConfirmHandler default value is using the raw
step.Default string, so templated defaults like {{ .env.* }} or !exec are never
resolved and always fall back to false. In ConfirmHandler.Execute, resolve
step.Default through vars.Resolve first (as done in
choose/input/file/filter/write) and then apply the yes/true check on the
resolved value before the non-TTY branch uses confirmValue(defaultVal).

---

Nitpick comments:
In `@pkg/runner/step/choose.go`:
- Around line 94-103: The same resolveDefault logic is duplicated in
ChooseHandler, FileHandler, FilterHandler, and WriteHandler, so move it into
BaseHandler alongside ResolvePrompt and make it handle the Default field with
the same empty-check, vars.Resolve call, and wrapped error. Update the handler
methods in choose.go, file.go, filter.go, and write.go to call
h.ResolveDefault(step, vars) instead of keeping private copies, and remove the
duplicated implementations.

In `@pkg/runner/step/interactive_default_test.go`:
- Around line 168-194: The non-TTY default-templating coverage is too narrow
because TestInteractiveHandlers_NonTTYDefaultTemplating only exercises the
choose handler. Refactor this test into a table-driven test like
TestInteractiveHandlers_NonTTYUsesDefault and run the same {{ .env.* }}
default-templating assertion across the relevant interactive handlers (input,
confirm, file, filter, write, and choose). Use Get("...") with each handler name
and verify Execute returns the rendered default value for each case so
templating regressions in handlers like confirm are covered.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e0ee244b-3e87-4943-8388-59319ac3aec3

📥 Commits

Reviewing files that changed from the base of the PR and between 4ee0f7f and 42993ba.

📒 Files selected for processing (23)
  • docs/prd/interactive-steps-non-tty-defaults.md
  • internal/exec/workflow_step_functions.go
  • internal/exec/workflow_step_functions_test.go
  • internal/exec/workflow_utils.go
  • pkg/runner/step/choose.go
  • pkg/runner/step/confirm.go
  • pkg/runner/step/file.go
  • pkg/runner/step/filter.go
  • pkg/runner/step/handler_base.go
  • pkg/runner/step/input.go
  • pkg/runner/step/interactive_default_test.go
  • pkg/runner/step/write.go
  • website/blog/2026-07-07-interactive-steps-ci-defaults.mdx
  • website/blog/authors.yml
  • website/docs/cli/configuration/commands/command/steps.mdx
  • website/docs/workflows/workflows/workflow/steps/index.mdx
  • website/docs/workflows/workflows/workflow/steps/type/choose.mdx
  • website/docs/workflows/workflows/workflow/steps/type/confirm.mdx
  • website/docs/workflows/workflows/workflow/steps/type/file.mdx
  • website/docs/workflows/workflows/workflow/steps/type/filter.mdx
  • website/docs/workflows/workflows/workflow/steps/type/input.mdx
  • website/docs/workflows/workflows/workflow/steps/type/write.mdx
  • website/src/data/roadmap.js

Comment on lines +70 to +73
t.Run("malformed env function returns an error", func(t *testing.T) {
_, err := resolveStepFunctionString(atmosConfig, "!env")
require.Error(t, err)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find what error processCustomTags returns for malformed !env
rg -n 'func processCustomTags\b' --type go internal/ pkg/ --glob '!*_test.go' -C 10

# Find error sentinels related to env/yaml function evaluation
rg -n 'Err.*Env\|Err.*Yaml\|Err.*Function\|Err.*Tag\|Err.*Custom' --type go errors/ --glob '!*_test.go'

Repository: cloudposse/atmos

Length of output: 3150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant implementation and surrounding error definitions.
sed -n '1,260p' internal/exec/yaml_func_utils.go
printf '\n---\n'
sed -n '1,220p' internal/exec/workflow_step_functions_test.go

printf '\n=== sentinels / error types ===\n'
rg -n 'Err[A-Z][A-Za-z0-9_]*|errors\.New|fmt\.Errorf\(' internal/ pkg/ --glob '!*_test.go' -C 2

Repository: cloudposse/atmos

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Narrow to the workflow-step function path and the error created for malformed tags.
rg -n 'resolveStepFunctionString|processCustomTagsWithContext|malformed|!env|invalid|error' internal/exec --glob '!*_test.go' -C 3

# Print the lines around processCustomTagsWithContext and any malformed-tag branches.
sed -n '260,520p' internal/exec/yaml_func_utils.go

Repository: cloudposse/atmos

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the implementation paths that drive the malformed !env error.
sed -n '170,360p' internal/exec/yaml_func_utils.go
printf '\n---\n'
sed -n '1,140p' internal/exec/workflow_step_functions_test.go
printf '\n---\n'
sed -n '1,220p' pkg/utils/yaml_utils.go

Repository: cloudposse/atmos

Length of output: 19962


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for any sentinel or wrapped error specifically used by the custom-tag resolver.
rg -n 'errors\.New|fmt\.Errorf|ErrorIs|errors\.Is|custom tag|malformed !env|!env' internal/exec pkg/ --glob '!*_test.go' -C 2

# Show the exact implementation of the function under test if it's short enough.
awk 'NR>=170 && NR<=260 {print NR ":" $0}' internal/exec/yaml_func_utils.go

Repository: cloudposse/atmos

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact resolver used by the test and its error handling.
rg -n 'func resolveStepFunctionString\b|func resolveWorkflowStepFunctions\b|ProcessTagEnv\(' internal/exec/workflow_step_functions.go internal/exec/yaml_func_utils.go pkg/utils/yaml_func_env.go -C 6

printf '\n--- workflow_step_functions.go ---\n'
awk 'NR>=1 && NR<=220 {print NR ":" $0}' internal/exec/workflow_step_functions.go

printf '\n--- yaml_func_env.go ---\n'
awk 'NR>=1 && NR<=140 {print NR ":" $0}' pkg/utils/yaml_func_env.go

Repository: cloudposse/atmos

Length of output: 13257


Use require.ErrorIs for this case. !env returns pkg/utils.ErrInvalidAtmosYAMLFunction, so assert that sentinel instead of only checking that an error occurred.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/exec/workflow_step_functions_test.go` around lines 70 - 73, The test
in resolveStepFunctionString should assert the specific sentinel error instead
of just any error. Update the “malformed env function returns an error” case to
use require.ErrorIs against pkg/utils.ErrInvalidAtmosYAMLFunction, keeping the
existing resolveStepFunctionString and atmosConfig setup intact.

Comment on lines +146 to +155
vars := stepExecutorState.Variables()
for _, env := range stepEnv {
key, value, ok := strings.Cut(env, "=")
if !ok {
continue
}
vars.SetEnv(key, value)
}
return vars.Resolve(command)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether Variables() returns a shared mutable object and how
# custom command steps handle env var templating for parity comparison.

# Find the Variables() method definition
rg -n 'func.*Variables\(\)' --type go internal/ pkg/ --glob '!*_test.go' -C 5

# Find the SetEnv method definition
rg -n 'func.*SetEnv\(' --type go internal/ pkg/ --glob '!*_test.go' -C 5

# Find stepExecutorState declaration and initialization
rg -n 'stepExecutorState' --type go internal/exec/ --glob '!*_test.go' -C 3

# Check how custom command steps handle env var templating
rg -n 'SetEnv\|\.env\.' --type go internal/exec/ --glob '!*_test.go' -C 3

Repository: cloudposse/atmos

Length of output: 10172


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the executor/variables implementation and the workflow step flow.
sed -n '1,240p' pkg/runner/step/variables.go
printf '\n---\n'
sed -n '1,220p' pkg/runner/step/executor.go
printf '\n---\n'
sed -n '760,840p' internal/exec/workflow_utils.go
printf '\n---\n'
sed -n '120,190p' internal/exec/workflow_step_functions.go

Repository: cloudposse/atmos

Length of output: 16856


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where resolveWorkflowStepCommand is called and how stepEnv is assembled.
rg -n 'resolveWorkflowStepCommand\(' internal/ --glob '!*_test.go' -C 4
printf '\n---\n'
rg -n 'stepEnv|envVars' internal/exec/ --glob '!*_test.go' -C 4
printf '\n---\n'
rg -n 'workflowCommandSupportsTemplating|RenderCommand|show\.command|render command' internal/ --glob '!*_test.go' -C 3

Repository: cloudposse/atmos

Length of output: 32969


Avoid reusing the shared env store here internal/exec/workflow_step_functions.go:146-151
SetEnv mutates stepExecutorState.Variables(), so a later step can resolve {{ .env.* }} from values left behind by earlier steps and leak them into the rendered command. Scope resolution to the current step only, or restore the env map after Resolve.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/exec/workflow_step_functions.go` around lines 146 - 155, The current
env handling in workflowStepFunction reuses stepExecutorState.Variables(), so
SetEnv mutates shared state and can leak values into later steps. Update the
logic in workflowStepFunction to scope env overrides only to the current command
rendering, either by working on a step-local copy of the Variables state before
calling Resolve or by restoring the original env map immediately after Resolve
completes. Keep the fix centered around the workflowStepFunction and its
Variables/SetEnv/Resolve flow so later steps do not see prior step env values.

Comment on lines 41 to +55
func (h *ConfirmHandler) Execute(ctx context.Context, step *schema.WorkflowStep, vars *Variables) (*StepResult, error) {
defer perf.Track(nil, "step.ConfirmHandler.Execute")()

if err := h.CheckTTY(step); err != nil {
useTTY, err := h.resolveInteractive(step)
if err != nil {
return nil, err
}

// Parse default value.
defaultVal := strings.ToLower(step.Default) == "yes" || strings.ToLower(step.Default) == "true"

// Non-TTY with a configured default: use the default without prompting.
if !useTTY {
return NewStepResult(confirmValue(defaultVal)), nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Confirm default is never templated — {{ .env.* }}/!exec defaults silently evaluate to false.

Every other interactive handler (choose, input, file, filter, write) resolves step.Default through vars.Resolve before use. confirm.go compares the raw, un-resolved step.Default string directly against "yes"/"true". A workflow author writing default: "{{ .env.AUTO_APPROVE }}" for a confirm step will always get false in CI, since the literal template string never matches. This is a silent correctness gap — no error, just the wrong (safe-looking but potentially wrong) answer. TestInteractiveHandlers_NonTTYDefaultTemplating only covers choose, so this slipped through.

🐛 Proposed fix
-	// Parse default value.
-	defaultVal := strings.ToLower(step.Default) == "yes" || strings.ToLower(step.Default) == "true"
+	// Parse default value (resolve templates first, e.g. {{ .env.VAR }}).
+	resolvedDefault := step.Default
+	if resolvedDefault != "" {
+		var resolveErr error
+		resolvedDefault, resolveErr = vars.Resolve(resolvedDefault)
+		if resolveErr != nil {
+			return nil, fmt.Errorf("step '%s': failed to resolve default: %w", step.Name, resolveErr)
+		}
+	}
+	defaultVal := strings.ToLower(resolvedDefault) == "yes" || strings.ToLower(resolvedDefault) == "true"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (h *ConfirmHandler) Execute(ctx context.Context, step *schema.WorkflowStep, vars *Variables) (*StepResult, error) {
defer perf.Track(nil, "step.ConfirmHandler.Execute")()
if err := h.CheckTTY(step); err != nil {
useTTY, err := h.resolveInteractive(step)
if err != nil {
return nil, err
}
// Parse default value.
defaultVal := strings.ToLower(step.Default) == "yes" || strings.ToLower(step.Default) == "true"
// Non-TTY with a configured default: use the default without prompting.
if !useTTY {
return NewStepResult(confirmValue(defaultVal)), nil
}
func (h *ConfirmHandler) Execute(ctx context.Context, step *schema.WorkflowStep, vars *Variables) (*StepResult, error) {
defer perf.Track(nil, "step.ConfirmHandler.Execute")()
useTTY, err := h.resolveInteractive(step)
if err != nil {
return nil, err
}
// Parse default value (resolve templates first, e.g. {{ .env.VAR }}).
resolvedDefault := step.Default
if resolvedDefault != "" {
var resolveErr error
resolvedDefault, resolveErr = vars.Resolve(resolvedDefault)
if resolveErr != nil {
return nil, fmt.Errorf("step '%s': failed to resolve default: %w", step.Name, resolveErr)
}
}
defaultVal := strings.ToLower(resolvedDefault) == "yes" || strings.ToLower(resolvedDefault) == "true"
// Non-TTY with a configured default: use the default without prompting.
if !useTTY {
return NewStepResult(confirmValue(defaultVal)), nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/runner/step/confirm.go` around lines 41 - 55, The ConfirmHandler default
value is using the raw step.Default string, so templated defaults like {{ .env.*
}} or !exec are never resolved and always fall back to false. In
ConfirmHandler.Execute, resolve step.Default through vars.Resolve first (as done
in choose/input/file/filter/write) and then apply the yes/true check on the
resolved value before the non-TTY branch uses confirmValue(defaultVal).

@sgtoj

Copy link
Copy Markdown
Contributor Author

Closing to iterate on the implementation in my fork (addressing review feedback: dedupe default resolution, remove dead CheckTTY, align workflow template engine with custom commands, non-mutating env resolution). Will reopen when ready.

@atmos-pro

atmos-pro Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Note

Atmos Pro  

Waiting for your GitHub Actions workflow to upload affected stacks.
Learn More.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l Large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant