fix(control): don't auto-answer 'ask' in yolo/bypass mode#3638
Closed
warvyvr wants to merge 3 commits into
Closed
Conversation
SivanCola
requested changes
Jun 9, 2026
SivanCola
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the fix — the production change matches the reported YOLO/ask bug, and the targeted tests pass locally.
Could you tighten TestAskSerializesBehindPromptLockEvenWithBypass before merge? Right now it locks promptMu, starts Ask, sleeps for 20ms, then releases the lock and waits for AskRequest. That verifies the request is eventually emitted under bypass, but it does not actually prove Ask serialized behind promptMu: if a future regression emitted AskRequest before taking the lock, this test could still pass because the buffered askCh would simply be read after unlock.
Please add a pre-unlock assertion that no AskRequest is observed while promptMu is still held, then unlock and assert the request is emitted. That would make the test cover the serialization behavior its name/comment describes.
Also noted while validating: go test ./internal/control currently fails locally in TestRemoveMCPServerRemovesUnconnectedLazyPlaceholder because my environment has configured MCP servers, but the changed yolo/ask tests and internal/agent ask tests pass.
The bypass flag was designed to skip permission approvals (requestApproval), but was incorrectly applied to Controller.Ask() — where the model asks the user a structured question, not where it asks permission. This meant yolo mode silently auto-selected the first option without the user ever seeing the prompt. Fix: remove both bypassEnabled() checks from Ask(). The function now always emits an AskRequest and waits for AnswerQuestion(), even under bypass. The second check (after promptMu.Lock()) was guarding a race where bypass was turned on while Ask was queued behind another prompt — that race no longer matters because bypass no longer suppresses Ask. Keep recommendedAskAnswers() as dead code with a doc comment explaining its history, for reference. Update tests: - TestBypassAutoAnswersAskWithRecommendedOptions → TestBypassDoesNotAutoAnswerAsk verifies Ask emits AskRequest under bypass and returns the user's actual choice - TestBypassRecheckedForAskAfterPromptLock → TestAskSerializesBehindPromptLockEvenWithBypass verifies Ask serializes behind promptMu and still emits AskRequest Fixes #3624
Contributor
Author
|
@SivanCola as your request, I change testcase, please check it out, for any further consideration, please let me know. |
- wow.md: bug 修复全流程指南（分类、复现、修改、验证）+ 注释语言规则 - README.md: 在 Features 前添加 wow.md 链接，引导新贡献者 - .gitignore: 添加 priority.md（本地 issue 跟踪，永不提交）
Owner
|
Thanks @warvyvr — your behavior tests (TestBypassDoesNotAutoAnswerAsk / TestAskSerializesBehindPromptLockEvenWithBypass) are great and are carried into #3712. Closing in favor of that, which keeps your tests + the minimal fix but drops the unrelated files this PR picked up (wow.md, README.md, .gitignore). Credited via Co-authored-by. Closes the same #3624. |
esengine
added a commit
that referenced
this pull request
Jun 9, 2026
) * fix(control): don't auto-answer the ask tool in YOLO mode Controller.Ask checked bypassEnabled() and silently returned the first option of every question, so in YOLO mode the `ask` tool got a fake answer and the user never saw the prompt. Bypass is meant to skip tool-approval gates, not to answer the user's genuine questions. Remove both bypass checks (and the now unused recommendedAskAnswers helper); Ask always emits an AskRequest and waits for AnswerQuestion, even under bypass. Combines the minimal fix from #3709 (CVEngineer66) with the behavior tests from #3638 (warvyvr), dropping that PR's unrelated files. Closes #3624 Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com> * fix(control): drop now-unused bypassEnabled helper Removing the bypass checks from Ask left bypassEnabled with no callers (staticcheck unused). --------- Co-authored-by: reasonix <reasonix@deepseek.com> Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com>
SuMuxi66
pushed a commit
to SuMuxi66/DeepSeek-Reasonix
that referenced
this pull request
Jun 10, 2026
) (esengine#3712) * fix(control): don't auto-answer the ask tool in YOLO mode Controller.Ask checked bypassEnabled() and silently returned the first option of every question, so in YOLO mode the `ask` tool got a fake answer and the user never saw the prompt. Bypass is meant to skip tool-approval gates, not to answer the user's genuine questions. Remove both bypass checks (and the now unused recommendedAskAnswers helper); Ask always emits an AskRequest and waits for AnswerQuestion, even under bypass. Combines the minimal fix from esengine#3709 (CVEngineer66) with the behavior tests from esengine#3638 (warvyvr), dropping that PR's unrelated files. Closes esengine#3624 Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com> * fix(control): drop now-unused bypassEnabled helper Removing the bypass checks from Ask left bypassEnabled with no callers (staticcheck unused). --------- Co-authored-by: reasonix <reasonix@deepseek.com> Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com>
dorokuma
pushed a commit
to dorokuma/DeepSeek-Reasonix
that referenced
this pull request
Jun 10, 2026
) (esengine#3712) * fix(control): don't auto-answer the ask tool in YOLO mode Controller.Ask checked bypassEnabled() and silently returned the first option of every question, so in YOLO mode the `ask` tool got a fake answer and the user never saw the prompt. Bypass is meant to skip tool-approval gates, not to answer the user's genuine questions. Remove both bypass checks (and the now unused recommendedAskAnswers helper); Ask always emits an AskRequest and waits for AnswerQuestion, even under bypass. Combines the minimal fix from esengine#3709 (CVEngineer66) with the behavior tests from esengine#3638 (warvyvr), dropping that PR's unrelated files. Closes esengine#3624 Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com> * fix(control): drop now-unused bypassEnabled helper Removing the bypass checks from Ask left bypassEnabled with no callers (staticcheck unused). --------- Co-authored-by: reasonix <reasonix@deepseek.com> Co-authored-by: CVEngineer66 <CVEngineer66@users.noreply.github.com> Co-authored-by: warvyvr <warvyvr@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bypass flag was designed to skip permission approvals (requestApproval), but was incorrectly applied to Controller.Ask() — where the model asks the user a structured question, not where it asks permission. This meant yolo mode silently auto-selected the first option without the user ever seeing the prompt.
Fix: remove both bypassEnabled() checks from Ask(). The function now always emits an AskRequest and waits for AnswerQuestion(), even under bypass. The second check (after promptMu.Lock()) was guarding a race where bypass was turned on while Ask was queued behind another prompt — that race no longer matters because bypass no longer suppresses Ask.
Keep recommendedAskAnswers() as dead code with a doc comment explaining its history, for reference.
Update tests:
Fixes #3624