Skip to content

Conversation

@JAORMX
Copy link
Collaborator

@JAORMX JAORMX commented Nov 17, 2025

Summary

Removes the StepTypeConditional constant and all related code that was added as a placeholder but never designed or implemented in the proposal.

Background

The constant was added in #2439 alongside the condition field implementation, but:

  • It was never documented in the THV-2106 proposal
  • No design exists for how it would work
  • It's been marked "not yet supported" since day one
  • It causes confusion by suggesting a feature that doesn't exist

What Changed

  • ✅ Removed StepTypeConditional constant from composer package
  • ✅ Removed conditional case from workflow execution switch
  • ✅ Removed conditional validation case
  • ✅ Removed conditional parsing from workflow converter
  • ✅ Updated step type documentation comment

What Stays the Same

The condition field on workflow steps remains unchanged and fully functional. This field enables conditional execution via template evaluation (e.g., condition: "{{.steps.confirm.output.action == 'accept'}}").

Testing

  • ✅ All composer tests pass
  • ✅ All vmcp tests pass
  • ✅ Linter passes with 0 issues
  • ✅ K8s CRD already has correct enum (tool;elicitation)
  • ✅ No e2e test changes needed

Notes

One test (TestDAGExecutor_ComplexWorkflow) shows intermittent flakiness with the race detector (timing-sensitive parallel execution test), but passes consistently without it. This is unrelated to these changes.

Removes the StepTypeConditional constant and all related code that was
added as a placeholder but never designed or implemented. This constant
caused confusion as it suggested a feature that doesn't exist.

The `condition` field on workflow steps (which enables conditional
execution via template evaluation) remains unchanged and fully functional.
This change only removes the unused step type constant.

Changes:
- Remove StepTypeConditional constant from composer package
- Remove conditional case from workflow execution switch
- Remove conditional validation case
- Remove conditional parsing from workflow converter
- Update step type documentation comment

All tests pass. The Kubernetes CRD already has the correct enum
(tool;elicitation) without the conditional type.
@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.85%. Comparing base (107ea07) to head (2ae8e70).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2612   +/-   ##
=======================================
  Coverage   55.85%   55.85%           
=======================================
  Files         312      312           
  Lines       29541    29531   -10     
=======================================
- Hits        16501    16496    -5     
+ Misses      11599    11593    -6     
- Partials     1441     1442    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX JAORMX merged commit a26f8af into main Nov 17, 2025
29 checks passed
@JAORMX JAORMX deleted the remove-steptype-conditional branch November 17, 2025 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants