fix: always pass prompt via stdin to satisfy cagent exec's required prompt arg#46
Conversation
…rompt arg Signed-off-by: Derek Misler <derek.misler@docker.com>
|
/describe |
|
✅ PR description has been generated and updated! |
There was a problem hiding this comment.
Review Summary
✅ APPROVED - No new bugs introduced
The refactoring successfully achieves its goal of always passing a prompt via stdin to satisfy cagent exec's required prompt argument. The changes consolidate the conditional logic into a simpler, unconditional flow without introducing new functional issues.
Changes Reviewed
- ✅ Always adds
-to ARGS to enable stdin input (line 498) - ✅ Unconditionally pipes prompt (or empty string) to stdin
- ✅ Simplified branching logic while maintaining functional behavior
- ✅ README.md documentation cleanup
Optional Future Enhancement
While not blocking this PR, I noticed pre-existing exit code handling that could be improved in a future change:
- Line 512:
EXIT_CODE=$?captures tee's exit code instead of the timeout command's exit code - Line 518:
EXIT_CODE=${PIPESTATUS[0]}captures printf's exit code instead of cagent's exit code
These issues existed before your changes and don't need to be addressed in this PR. The consolidation you've done is correct. Consider ${PIPESTATUS[1]} for proper exit code capture in both cases if you want to tackle this separately.
Verdict: The changed code works correctly for its intended purpose. Ship it! 🚢
Summary
Fixes a bug where
cagent execwould fail when no prompt was provided, since the CLI requires a prompt argument. The action now always passes-(stdin) as the prompt argument, sending an empty string when no prompt is specified.Changes
-to arguments and always pipes prompt via stdin (empty string if not provided), eliminating conditional branching for prompt handlingBreaking Changes
None
How to Test
cagent execexecutes successfully with an empty prompt