-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Make confirm_ask
behavior consistent with explicit confirmation handling
#3773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…t_yes_required correctly
Thanks for your interest in aider and for taking the time to make this PR. It sounds like you are fixing a bug? If so, can you add a test case which triggers the bug? The test should fail in the current code base and pass in the new code base. |
Hi @paul-gauthier, With the updated changes, even if self.yes is true, specifying explicit_yes_required forces the prompt to appear, allowing the user to manually confirm or deny the action. This adjustment ensures that critical commands (like those involving shell commands) are not executed without a deliberate user decision. I’ve also updated/modified the unit tests to expose this behavior. The tests, which initially failed by demonstrating the lack of proper prompting and action execution, now pass under the corrected logic. ![]()
Would you like me to add these tests to the main branch before merging the PR? Thanks for your guidance, and I look forward to your feedback. |
The intended behavior is to ignore/deny confirmations in that situation. If the user has specified --yes, they often intend to operate without supervising aider. |
@paul-gauthier, thanks for clarifying. I now understand that the intention with –-yes-always is to operate fully non-interactively and ignore confirmations. With that in mind, would it make sense to have an optional parameter (for example, –-ask-for-explicit-yes) that forces the prompt for waiting/dangerous actions even when –-yes-always is active? Or should we keep the current behavior as it is? Let me know what you think but I think this pr is good to be closed. Thanks Again 🥇 |
This PR refactors the confirm_ask function to ensure consistent behavior when handling the explicit_yes_required parameter. The changes include:
Improved Logic: The function now handles cases where self.yes is True, False, or None more predictably, ensuring that user confirmation is appropriately prompted or bypassed based on the context.
Removed Unused Code: The unused valid_responses logic has been removed, simplifying the code and improving maintainability.
Enhanced Use Case Support: This update is particularly useful in scenarios where commands suggested by an LLM require manual confirmation even when --yes-always flag is set. It ensures that users are prompted for confirmation when necessary, rather than skipping the confirmation step unintentionally.
These changes improve the clarity, reliability, and usability of the confirm_ask function.