Fire subagent queries by default and bound the sync wait - #206
Merged
Merged
Conversation
The query_subagent tool now defaults to wait = FALSE: the prompt starts at once and collect_subagent fetches the reply, so several subagents can work in parallel while the parent continues. The tool gains a timeout argument for the cases that do wait. subagent_query(wait = TRUE) used session$run(), which has no timeout, so a child that never replied held the parent's turn indefinitely. Both paths now fire the same one-shot session$call() and the sync path collects through subagent_collect() with a real deadline. A child that has not replied by then stays pending for a later collect or kill and the call returns NULL. subagent_collect() maps Inf to processx's wait-forever sentinel instead of NA. /ask reports the bounded wait running out and points at /collect; the hall monitor reads NULL as no verdict and escalates, as its contract already said. Tests drive the sync path with a fake r_session: a slow child leaves the slot pending and the poll receives the deadline in milliseconds, a fast child returns directly, Inf becomes -1, and the tool's default and messages hold.
tool_query_subagent() takes timeout after return_name, so a positional caller passing return_name fourth still reaches the child with it instead of waiting on a string. .subagent_poll_ms() validates timeout before a query is fired: one non-negative number of seconds or Inf, or an error. NA, NaN, strings, and negatives used to fall through to processx's -1 sentinel and wait forever. Inf, and finite values too large for integer milliseconds, map to that sentinel on purpose. /ask keeps its three outcomes apart inside the handler: a reply, a bounded wait running out (NULL from a successful call), or an error, which prints once and never leads to a second lookup outside the handler. .subagent_still_pending() is gone with it. Docs say the wait is bounded by default rather than never indefinite, since Inf opts out.
A finite timeout longer than about 24.8 days used to map to processx's -1 sentinel and wait forever, which contradicts the bound the caller asked for. It is now an error that points at Inf, the one value that opts out of the bound.
# Conflicts: # DESCRIPTION # NEWS.md
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
1 participant
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.
Summary
query_subagentdefaults towait = FALSEand gainstimeout: the prompt starts at once andcollect_subagentfetches the reply, so several subagents can run in parallel while the parent continues. The tool description tells the model so.subagent_query(wait = TRUE)no longer callssession$run(), which has no timeout. Both paths fire onesession$call(); the sync path collects with a real deadline (timeout, default 60 s,Infallowed). A child that has not replied stays pending forsubagent_collect()orsubagent_kill()and the call returns NULL instead of holding the parent's turn.subagent_collect()mapsInfto processx's-1rather than NA./askin the REPL reports a bounded wait running out and points at/collect. The hall monitor reads a NULL reply as no verdict and escalates, which its contract already stated.Test plan
inst/tinytest/test_subagent_async.Rdrives the sync path with a faker_session: slow child leaves the slot pending with the poll receiving 10 ms, fast child returns directly,Infbecomes-1, tool default is FALSE and its queued / still-working messages hold. 37 asserts.test_mcp_handler.R:124, the known order-dependent check that passes alone and fails the same way on main.test_subagent_callr.R(real callr child, real model) passed all 10.