fix(delegation): surface a child's undelivered steer instead of dropping it - #76805
Conversation
Related: #70899 already exposes ID-addressed live-child steering in the TUI, while #76512 proposes parent-scoped model controls. This smaller |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused live-child steering seam. The premise is real on current main: tools/delegate_tool.py:183-216 and tui_gateway/methods_session.py:2806-2814 provide interruption only.
Problems
tools/delegate_tool.py(proposedreturn bool(agent.steer(text))) can acknowledge a steer that never reaches the child.AIAgent.steer()queues text without checking for a remaining injection boundary (run_agent.py:3153-3169); the finalizer returns late text aspending_steer(agent/turn_finalizer.py:680-685), but_run_single_childdoes not retain it in its completion entry (tools/delegate_tool.py:2310-2417).- The proposed RPC is unscoped: current registry records have no owner/session field (
tools/delegate_tool.py:2092-2107), and the endpoint accepts only an ID and text. The existing discussion correctly identifies #70899 and #76512 as requiring an explicit authority/RPC decision.
Suggested changes
- Handle the final-answer race with a truthful missed/undelivered result and regression coverage.
- Settle and implement the authority contract before exposing the RPC.
- Add
server.handle_requestcoverage for RPC validation and result shapes, alongside the helper tests.
This is an automated hermes-sweeper review.
| return False | ||
| try: | ||
| return bool(agent.steer(text)) | ||
| except Exception as exc: |
There was a problem hiding this comment.
AIAgent.steer() accepts by queuing _pending_steer, even after the final tool boundary; the turn finalizer then returns it as pending_steer. _run_single_child currently drops that field when it builds the completion entry, so this can return True for guidance the child never sees. Please make the final-answer race observable or reject it before reporting acceptance.
| @@ -2814,6 +2814,20 @@ def _(rid, params: dict) -> dict: | |||
| return _ok(rid, {"found": ok, "subagent_id": subagent_id}) | |||
|
|
|||
|
|
|||
| @method("subagent.steer") | |||
ebebbe1 to
cc3e332
Compare
|
Both problems addressed in cc3e332c8 (rebased onto current main):
|
cc3e332 to
6f6feff
Compare
|
Housekeeping on the comment above: after posting it I squashed the branch to a single commit per the contributing guide's one-logical-change-per-PR rule and corrected the conventional-commit scope ( Related context, since it bears on how you'd want to take this: I've opened #77111, an RFC proposing a This PR stands on its own — |
6f6feff to
a9ca4fb
Compare
Say 'tell that audit to focus on the token refresh instead' and the note reaches the agent after its next tool call. Not an interrupt: the current step always finishes, and a child already past its last tool call is reported as such rather than left to look like it landed. Only the attached lane can carry a steer, so the other two answer with their own reason instead of a generic failure — the api_server lane exposes stop and nothing else, and a detached one-shot has no inbound channel at all. Both offer the thing that lane CAN do. On the attached lane it prefers the host's own steer_subagent tool (NousResearch/hermes-agent#76805) and falls back to resolving the same delegation registry directly, since AIAgent.steer() and _active_subagents have shipped in main far longer than the tool that addresses a child by id. A genuine host error — paused delegation, a depth limit — is spoken, never routed around. 23 tests, one per lane and per bridge branch.
a9ca4fb to
3488323
Compare
|
@teknium1 this one's ready for eyes. Rebased on current main, CI green (38 checks), +296/-1. Short version: This completes that contract for delegated children: |
…ing it The turn finalizer already hands back steer text that queued after the final tool batch — result["pending_steer"], with the comment "hand it back to the caller so it can be delivered as the next user turn instead of being silently lost." Every interactive surface honors that contract (cli.py, gateway/run.py, tui_gateway/server.py all requeue it). The delegation layer doesn't: _run_single_child never reads it, so a steer queued into a delegated child that finishes first vanishes with no trace in the completion entry. There is also no sanctioned sender: the registry has interrupt_subagent() but no redirection-side mirror, and session.steer cannot reach children (lazy watch sessions have agent=None, so it 4010s). Complete the contract for delegated children — both halves: - steer_subagent(subagent_id, text): redirection-side mirror of interrupt_subagent(). Resolves the live child in _active_subagents and queues text via AIAgent.steer(). True means queued, not delivered. - missed_steer retention: when the child's result carries pending_steer, _run_single_child names it on the completion entry (missed_steer field plus a summary note) so the parent can re-issue the guidance instead of trusting it landed. This is what makes adding a sender safe: without it the finish-before-drain race silently loses the text — the exact loss the finalizer contract exists to prevent. - subagent.steer gateway RPC beside subagent.interrupt so programmatic hosts (dashboard, voice layers, ACP bridges) get an in-tree caller; catalogued in programmatic-integration.md. - docs: "Steering a Running Subagent" section in delegation.md covering the queued-vs-delivered semantics. Tests: registry-level steer coverage (delivery, unknown id, empty text, dead record, raising agent), the finish-before-drain race retaining missed_steer, and the RPC contract (4000/4002 validation, queued and rejected envelopes).
3488323 to
0c84ba7
Compare
|
Final remediation is complete on
Evidence: 30 focused steering tests, 518 gateway tests, 151 broad delegate/async tests, and 118 independent lifecycle/ownership tests passed; Ruff/compile/diff checks passed; independent final QA found no blocker/major. |
The dropped signal
agent/turn_finalizer.py:683-685already guards against losing a steer that queues after the final tool batch:Every interactive surface honors that contract —
cli.py,gateway/run.py, andtui_gateway/server.pyall requeue the leftover text as the next user turn.The delegation layer doesn't.
_run_single_childintools/delegate_tool.pynever readsresult["pending_steer"], so the completion entry the parent receives carries no trace of it. And there's no sanctioned sender either: the registry exposesinterrupt_subagent()but no redirection-side mirror, andsession.steercan't reach a delegated child — the desktop opens children as lazy watch sessions withagent=None, so it returns 4010.Net: you can kill a running child, but you can't redirect one — and if you could, the finish-first race would silently lose the text, which is exactly the loss the finalizer contract exists to prevent.
The fix — both halves of the contract
Sender —
steer_subagent(subagent_id, text)intools/delegate_tool.py, the redirection-side mirror ofinterrupt_subagent(): resolves the live child in_active_subagentsand queues text viaAIAgent.steer(). The in-flight tool call is never cut; the child sees the text as an out-of-band user message at its next iteration boundary.Truemeans queued, not delivered. Fronted by asubagent.steergateway RPC besidesubagent.interrupt(tui_gateway/methods_session.py) so programmatic hosts — dashboard, voice layers, ACP bridges — have an in-tree caller. Subagent ids come fromdelegation.status, same assubagent.interrupt.Receiver — missed-steer retention in
_run_single_child: when the child's result carriespending_steer, the completion entry retains it asmissed_steerwith a note appended to the summary:The parent can tell a steered child from one that finished on the old instructions, and re-issue the guidance instead of trusting it landed.
Docs
website/docs/user-guide/features/delegation.md(queued-vs-delivered semantics, the race, themissed_steercontract)subagent.steeradded to the method catalog inwebsite/docs/developer-guide/programmatic-integration.mdTests
tests/tools/test_subagent_steer.py:False, never an exceptionpending_steerset produces a completion entry carryingmissed_steer+ the summary note,statusunaffected; a clean run leaves the entry untouchedsubagent_id), 4002 (emptytext),queuedandrejectedenvelopesAll green on the touched paths:
tests/tools/test_subagent_steer.py(12),tests/tools/test_delegate.py+tests/run_agent/test_steer.py(89),tui_gatewayprotocol + delegation lifecycle (52).