fix(composer): expand template expressions in elicitation messages#4313
fix(composer): expand template expressions in elicitation messages#4313amirejaz merged 3 commits intostacklok:mainfrom
Conversation
jerm-dro
left a comment
There was a problem hiding this comment.
Thanks for picking this up! Two requests before merging: avoid mutating the step definition (use an immutable assignment pattern), and restructure the test as table-driven with a few more cases. Details in the inline comments.
|
Thanks for the review — I’ve gone through the requested changes and queued follow-up updates. I’ll push the revision in a dedicated follow-up pass. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4313 +/- ##
==========================================
+ Coverage 69.53% 69.57% +0.03%
==========================================
Files 480 480
Lines 48641 48675 +34
==========================================
+ Hits 33823 33865 +42
+ Misses 12215 12201 -14
- Partials 2603 2609 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Pushed an update for both points. The step config stays immutable now (message expansion happens on a copied elicitation config), and the integration coverage is table-driven with both templated and plain message cases. Greetings, saschabuehrle |
Bug
#4312 — Template expressions like
{{.params.owner}}in elicitation stepmessagefields are passed through as raw placeholders instead of being expanded.Fix
Adds template expansion for
step.Elicitation.MessageinexecuteElicitationStep()before passing it toRequestElicitation(). This mirrors howexecuteToolStep()already expandsstep.Argumentsthrough the template expander (line ~404).The expansion wraps the message string in a temporary map to reuse the existing public
TemplateExpander.Expand()interface, keeping the change minimal and consistent with the established expansion pattern.Testing
TestWorkflowEngine_ElicitationMessageTemplateExpansionintegration test that verifies{{.params.repo}}and{{.params.env}}placeholders in an elicitation message are expanded to their resolved values before reaching the SDK elicitation requester.Happy to address any feedback.
Greetings, saschabuehrle