fix: push step condition to children to fix evaluation with artifacts

[Copilot]

Pull request description

Step conditions like env.TEST are incorrectly evaluated as false when the step also defines artifacts, causing the entire step to be skipped.

Root cause: When a step has multiple operations (shell + artifacts), the processor wraps them in a non-flattenable group. The group's Start action evaluates the condition before the Container transition that loads scoped env vars (e.g., _0_TEST=true → TEST=true). So os.Getenv("TEST") returns "" at evaluation time. With a single operation (no artifacts), the group flattens and the Container transition naturally precedes the condition check.

# Works (single operation → group flattens)
- condition: env.TEST
  shell: echo "test"

# Broken (two operations → group doesn't flatten, condition evaluated too early)
- condition: env.TEST
  shell: echo "test"
  artifacts:
    paths: ["*.xml"]

Fix: Set the group condition to literal "true" so it always starts, and push the step condition down to each child operation via AppendConditions. Children evaluate the condition after their Container transition loads env vars.

• processor.go: Group gets "true" when step has a condition; children receive the actual condition via AppendConditions (preserving operation-set conditions like "always" for artifacts)
• groupstage.go: Virtual group Flatten() now propagates its condition to children, so nested step conditions aren't lost
• processor_test.go: Regression test verifying condition ordering with artifacts

Checklist (choose whats happened)

• breaking change! (describe)
• tested locally
• tested on cluster
• added new dependencies
• updated the docs
• added a test

Breaking changes

Changes

• Group stages with step conditions now use literal "true" and push the condition to children
• Virtual group flattening propagates conditions to children before returning them

Fixes

• Step condition incorrectly evaluated as skipped when step contains artifact definitions

[vsukhin]

@greptile

[greptile-apps]

Greptile Summary

This PR fixes a bug where step conditions (e.g., condition: env.TEST) were always evaluated as false when the step also defined artifacts, causing the step to be incorrectly skipped. The root cause was that a multi-operation group's Start action evaluated the condition before the Container transition that loads scoped env vars. The fix sets the group condition to "true" so it always starts, then pushes the real condition down to each child operation via AppendConditions, where it is evaluated after the env var transition — a correct and well-targeted approach.

A secondary pre-existing bug is also silently fixed: the old SetCondition call was overwriting operation-level conditions like "always" on artifact steps; AppendConditions now correctly combines them.

Confidence Score: 5/5

Safe to merge — the fix is well-reasoned, targeted, and backed by a regression test. Only P2 findings remain.

No P0 or P1 issues found. The single P2 comment flags missing test coverage for the false-condition path, which is a non-blocking improvement. The core logic is correct and the changes are minimal in scope.

No files require special attention; the P2 is a test-coverage suggestion on processor_test.go.

Important Files Changed

Filename	Overview
pkg/testworkflows/testworkflowprocessor/processor.go	Core fix: group condition changed from the step condition to literal "true"; step condition pushed to children via AppendConditions (preserving operation-level conditions like "always"). Semantics when condition is false shift from "group skipped" to "group started, children skipped" — intentional but untested on the false path.
pkg/testworkflows/testworkflowprocessor/stage/groupstage.go	Flatten() now propagates a virtual group's condition to its children before dissolving the group. Children are fully flattened before propagation, so no double-application risk. Correct and targeted fix.
pkg/testworkflows/testworkflowprocessor/presets/processor_test.go	Adds a regression test that verifies group condition is "true" and children carry env.TEST after the fix. Covers only the happy path (TEST=true); the false-condition path is not tested.

Sequence Diagram

sequenceDiagram
    participant P as processor.go
    participant G as GroupStage (group)
    participant S as ShellStage (child)
    participant A as ArtifactsStage (child)
    participant E as Execution Engine

    Note over P,E: Step with condition:"env.TEST" + shell + artifacts

    P->>G: SetCondition("true")
    P->>S: AppendConditions("env.TEST")
    P->>A: AppendConditions("env.TEST")
    Note over A: combined: "env.TEST&&always"
    P->>G: Add(S), Add(A)

    E->>G: Declare (condition:"true")
    E->>G: Start (always fires)
    E->>S: Container transition (loads _0_TEST→TEST)
    E->>S: Evaluate condition "env.TEST" — correct timing
    E->>A: Evaluate condition "env.TEST&&always" — correct timing

Loading

_{Reviews (2): Last reviewed commit: "fix: go mod" | Re-trigger Greptile}

[vsukhin]

@copilot revert go.sum

[Copilot]

@copilot revert go.sum

Reverted in c609bc8.