feat(tools): add ask_user_question for mid-turn question prompting - #80
Merged
Merged
Conversation
Every mid-turn round-trip so far was yes/no/deny dangerous-tool
approval; there was no way for the model to ask the human an actual
question and get an answer back. Add ask_user_question, a
PermissionSafe tool that blocks on the human's reply the same way
tool approval already blocks, and returns the answer (a chosen
option's text, or free-typed text) as the tool result.
It reuses approvalRouter's pending-channel map instead of adding a
second one — one entry per session is safe because SessionLock already
limits a session to one in-flight tool call at a time. wait() now
takes the value to return on cancellation as a parameter ("deny" for
approval, "" for a question) instead of hardcoding "deny", since an
unanswered question isn't a denial.
modules/ask_question takes an injected AskFunc closure rather than
importing core, mirroring web_search/web_fetch's WebBackendLookup
pattern, since core already imports it and a direct import would
cycle. The REPL client gains renderer.readLine (Enter submits,
Backspace edits, Ctrl+C aborts) built on the same raw-byte primitive
the existing y/a/N approval prompt uses, and renderer.ask, which
accepts either a numbered option or free-typed text as the answer.
The -p --format json/xml path and a nil AskFunc both fail fast instead
of hanging, mirroring how approval_request is already auto-denied
there.
…tch in USAGE.md The ARCHITECTURE.md paragraph added for ask_user_question conflated two separate fallbacks into one: a nil AskFunc (which errors immediately) and renderer.collect's auto-reply for -p --format json/xml (which is a live /ws round trip that succeeds with an empty answer, not an error). Split them apart and named the one production call site that actually passes a nil ask (core/context.go, sizing a context window, never running a turn). USAGE.md's tool list never mentioned web_search/web_fetch (missed when they were added) or ask_user_question, and its "works without a cwd" list was stale against what buildTools actually gates on root != "" (only bash_exec and the file tools).
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
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.
What this changes
Every mid-turn round-trip so far was yes/no/deny dangerous-tool approval — there was no way for the model to ask the human an actual question and get an answer back. Adds
ask_user_question, aPermissionSafetool that blocks on the human's reply the same way tool approval already blocks, and returns the answer (a chosen option's text, or free-typed text) as the tool result.It reuses
approvalRouter's pending-channel map instead of adding a second one — one entry per session is safe becauseSessionLockalready limits a session to one in-flight tool call at a time.wait()now takes the value to return on cancellation as a parameter ("deny"for approval,""for a question) instead of hardcoding"deny", since an unanswered question isn't a denial.modules/ask_questiontakes an injectedAskFuncclosure rather than importingcore, mirroringweb_search/web_fetch'sWebBackendLookuppattern, sincecorealready imports it and a direct import would cycle. The REPL client gainsrenderer.readLine(Enter submits, Backspace edits, Ctrl+C aborts) built on the same raw-byte primitive the existing y/a/N approval prompt uses, andrenderer.ask, which accepts either a numbered option or free-typed text as the answer. The-p --format json/xmlpath and a nilAskFuncboth fail fast instead of hanging, mirroring howapproval_requestis already auto-denied there.How it was verified
make test-racepassesAdded
modules/ask_question/ask_question_test.go(missing-question error, nil-AskFuncerror, answer round-trip) andserver/sock/question_test.go(a fullquestion_request/questionround trip over a real websocket connection through the tool loop, mirroring the existing approval round-trip tests inserver/sock/hil_test.go).Checklist
:=, onevarblock per function in first-use order witherrlast, callees before callers, andmainunconditionally last.gofiles carry the two-line SPDX headerdocs/ARCHITECTURE.md)GPL-3.0-only, matching the project