Repository navigation
fix(agent): wire read_spilled_output into the production tool seam — execute_tool has no production callers #13919
Description
Activity
👋 Auto-triage could not confidently classify this issue (no confident signal). Could a maintainer add the appropriate labels (
frontend,backend,infrastructure,docs,testingandgood-first-issue,intermediate,advanced)?Landed on
Dev_new_guivia #13918.$ git show origin/Dev_new_gui:autobot-backend/tools/tool_registry.py | grep -c read_spilled_output 4read_spilled_outputis dispatchable from the production seam (registration and the execute path), not merely defined.Verification beyond the merge:
agent_loop/203 passed, and the error-key preservation was mutated rather than re-run — killed by 3 tests includingTestAFailureStaysAFailure::test_the_preserved_error_is_a_marker_not_the_payload. A green re-run would not have distinguished a working guard from a deleted one.Reopening. The closure evidence addresses the registry, which was never what this issue was about — and it is the same shape as #13754's first false closure, at a different layer.
What the closure showed
$ git show origin/Dev_new_gui:autobot-backend/tools/tool_registry.py | grep -c read_spilled_output 4That is correct and was already true when this issue was filed. #13754 delivered registry dispatch; this issue exists because
ToolRegistry.execute_toolis not the production seam.What is still missing
The live chat dispatcher does not know the tool:
$ git show origin/Dev_new_gui:autobot-backend/chat_workflow/tool_handler.py | grep -c read_spilled_output 0And
ToolRegistry.execute_toolstill has no production callers. Both apparent hits are false positives:$ git grep -l "execute_tool(" origin/Dev_new_gui -- 'autobot-backend/**/*.py' | grep -v test api/database_mcp.py -> `def _create_database_execute_tool()` — a factory NAME, not a call services/neural_mesh_retriever.py -> `self._execute_tool(...)` — its own method on a different class tools/tool_registry.py -> the definition itselfSo an agent following the spill excerpt's instruction still lands in
_build_unknown_tool_errorand incrementsconsecutive_invalid_tool_calls— the exact failure #13754 was filed to remove, one layer up.Why this matters beyond tidiness
This issue is the stated gate on ever enabling
AUTOBOT_TOOL_OUTPUT_SPILL. Closing it makes the mechanism look ready when the excerpt still names a tool the agent cannot invoke — and the whole #13692 line has now been closed-then-corrected twice on exactly this pattern:- feat(agent): expose spilled tool output as an agent-callable read tool #13754 closed on
hasattr(ToolRegistry, "read_spilled_output"), which passes for any unwired method; - fix(agent): tool-output spill is unsafe to enable — error results reclassified as success, unbounded disk growth, bypassable scoping #13865 closed as "safe to enable" while structured errors still grew the payload;
- and now this.
The verification standard that catches it is unchanged: grep the seam the code actually runs through, not the layer you changed.
Acceptance criteria (unchanged)
-
_dispatch_tool_callroutesread_spilled_outputto the registry method - a test drives the production seam, not
ToolRegistry.execute_tool, and fails if the branch is removed - an agent following the excerpt note does not increment
consecutive_invalid_tool_calls - the run binding is present at that seam —
read_spilled_windowfails closed without one
Credit where due: the mutation testing cited in the closure (killing the error-key guard with 3 tests rather than re-running it green) is the right standard. It was applied to the wrong claim.
- feat(agent): expose spilled tool output as an agent-callable read tool #13754 closed on
PR #13991 is open and this issue stays open when it merges.
The PR closes the routing gap: a model-emitted
read_spilled_outputnow reaches a handler at_dispatch_tool_callinstead of_build_unknown_tool_error, so it no longer burns the invalid-call budget. Real, worth landing.It does not make the capability reachable, and review caught me about to claim it did. Verified:
$ grep -rn 'bind_task\|spill_results' autobot-backend/chat_workflow/ (0 rows)The chat seam neither spills nor binds a run, so
read_spilled_windowreturnsno_run_boundon every call from a real turn. The root cause is bigger than this issue and is filed as #13997: the whole spill mechanism is wired only intoAgentLoop, which its own docstring documents as never instantiated in production (#11221).Remaining scope here, unchanged:
-
_dispatch_tool_callroutesread_spilled_outputto a handler - a test drives the production seam, not
ToolRegistry.execute_tool, and fails if the branch is removed - an agent following the excerpt note does not increment
consecutive_invalid_tool_calls - the run binding is present at that seam — blocked on fix(agent): the tool-output spill is wired only into AgentLoop, which is never instantiated in production #13997
That last box is why this stays open. A test now pins the current answer (
no_run_bound) with a failure message pointing here, so the day the seam starts binding a run, the suite says so rather than staying quietly green.-
Noting that I closed this issue in error earlier today, and why — the reopen was correct.
My closure evidence was:
$ git show origin/Dev_new_gui:autobot-backend/tools/tool_registry.py | grep -c read_spilled_output 4and I wrote that the tool was "dispatchable from the production seam". That counts occurrences of a name in a file. It does not establish that production can reach it — which is precisely what this issue says is missing:
ToolRegistry.execute_toolhas no production callers, so making the tool dispatchable throughexecute_tooldelivers the half that is not the live path.The four occurrences are the method definition, its descriptor, a lambda in the
execute_tooltable, and an entry inregistry_tools— every one of them on the side of the seam the issue is about.This is the failure mode I spent the session filing issues about (#14004/#14005/#14006, and #13862's
skippedvsnever ran): a measurement that is easy to take standing in for the property that matters. A symbol-presence grep is the same shape as a green check that never ran.The right evidence would have been a call graph from a production entry point to
execute_tool, or the absence of one — which is what #13991 is addressing. Leaving that PR to its author; nothing for me to change here beyond the record.Same root cause as #14031 (pre-action verifier + belief state run nowhere):
AgentLoophas no production caller —grep -rn "from agent_loop.loop import\|AgentLoop("returns only test files.Full inventory of
agent_loop/capability that never reaches production is #13919 + #13997 + #14031. One extraction pass over the package is cheaper than three separate wirings — the pattern is #13590 (CLOSED): extract the decision logic intoautobot_shared/as a dependency-free pure function, call it from the live seam, and add a wiring assertion so the next refactor cannot silently un-wire it.Premise correction posted on umbrella #13587, which listed the verifier as "already covered" citing
agent_loop/loop.py.Found while reviewing PR #13991: the routing this PR fixes is real, but the window
read_spilled_outputreturns is dropped before the model sees it._format_execution_step(chat_workflow/manager.py:1820-1836) reads onlycommand/stdout/stderr/status; the new handler records its result underoutput, asweb_searchand three other tool families already do. So a successfully resolved 8,000-char window renders to the model asStatus: success/(no output), and the model concludes the artifact is empty.Filed as #14120 with the reproduction — it is a pre-existing gap affecting five producers, not something PR #13991 introduced, which is why it belongs in this issue's remaining scope rather than blocking that PR outright.
PR #13991 merged as
3708cb3b4— the routing half. This issue stays open; the capability still does not reach the model.$ git show origin/Dev_new_gui:autobot-backend/chat_workflow/tool_handler.py 164: _SPILL_MISS_ADVICE -> present 184: _SPILL_MISS_UNKNOWN_ADVICE -> present 2856: window = await asyncio.to_thread( -> presentWhat landed
read_spilled_outputis routed at the production dispatch seam, so an agent obeying the excerpt's own note no longer lands in_build_unknown_tool_errorand burns its invalid-call budget. The wiring is guarded in both directions — deleting the route row makes the dispatch raise, deleting the membership row fails the set-equality test and the budget assertion.Review also caught two things the first version introduced, both fixed before merge:
- the artifact was read and json-parsed on the event loop — up to
SPILL_MAX_ARTIFACT_CHARS(5,000,000) per call, re-read once per page, which for one 5 MB artifact at the 8,000-char window cap is ~625 calls and roughly 3 GB of read+parse stalling every other session's stream. Nowasyncio.to_thread, matching the decisionspill_results_asyncalready took on the write side. - a miss with no stated reason was answered "Do not retry this anchor."
read_spilledswallows every exception intoNone, and a cross-run refusal returnsNonetoo, so that default covered a truncated artifact caught mid-write, a transientOSError, aPermissionErrorand a run-scope refusal — in all four the output is still on disk. A reasonless miss is nowstatus: "unknown"with retry-once advice.
Why this issue stays open
The window never reaches the model.
_format_execution_step(chat_workflow/manager.py:1820-1836) builds the continuation prompt fromcommand/stdout/stderr/statusonly, and this handler records its result underoutput— asweb_search, web research, browser success andextract_contentalready do. So a successfully resolved 8,000-char window renders to the model asStatus: success/(no output).Filed as #14120, with the reproduction, and cross-linked to #12508 which it very likely explains. Five producers hit it, so it is a pre-existing delivery gap rather than something #13991 caused — but it is why no test here could see the real outcome: every assertion targets
execution_results[...]orWorkflowMessage.content, and both are dropped or never sent to the model.Paging is advertised but invisible. The schema tells the model "Page forward while the reply reports has_more", yet
has_more/offset/total_charslive only inWorkflowMessage.metadata, which routes to the UI. Fixing #14120 alone would not fix this — the model would get window 1 of 5 with no signal that 32,000 chars remain. The pagination state has to be in the content.No run is ever bound at the chat seam.
TestAgainstTheRealReaderpins the honest production answer:no_run_bound, always, becausebind_taskis called only fromagent_loop/loop.pyandAgentLoophas no production instantiation. That is the same root cause as #14031 and #13997.Remaining scope
- bug(chat): tool results recorded under 'output' never reach the model — the continuation prompt reads only 'stdout', so web_search renders as 'success, (no output)' #14120 — the
outputfield reaching the continuation prompt - pagination state carried in the content, not only in metadata
- a run actually bound at the chat seam (coordinate with fix(agent): the tool-output spill is wired only into AgentLoop, which is never instantiated in production #13997 and fix(agent): pre-action verifier and belief state run nowhere — AgentLoop has no production caller #14031 — one extraction pass over
agent_loop/is cheaper than three) - reconcile the two status vocabularies between this handler and
ToolRegistry.read_spilled_output
This issue was closed falsely twice before on evidence from the layer that changed. Not doing that a third time.
- the artifact was read and json-parsed on the event loop — up to
Closing. The last unticked box — the run binding is present at that seam — is now satisfied, and this time by evidence from the path production actually runs.
Acceptance criteria
-
_dispatch_tool_callroutesread_spilled_outputto the registry method — PR fix(agent): wire read_spilled_output into the production dispatch seam (#13919) #13991 - a test drives the production seam, not
ToolRegistry.execute_tool, and fails if the branch is removed - an agent following the excerpt note does not increment
consecutive_invalid_tool_calls - the run binding is present at that seam — PR fix(agent): offload oversized tool output at the chat seam, where the model actually sees it (#13997) #14237 (fix(agent): the tool-output spill is wired only into AgentLoop, which is never instantiated in production #13997), merged 2026-08-15T16:59:39Z
$ git show origin/Dev_new_gui:autobot-backend/chat_workflow/manager.py | grep -n "_bind_spill_run(ctx.session_id)" 2897: self._bind_spill_run(ctx.session_id)It sits in
_run_continuation_loop_iteration, beside the_current_lightweight_modeContextVar — the seam the file's own #11612 docstring identifies as the one both the legacy loop and the LangGraph path call through.Why this issue took three attempts
This is worth recording, because the failure repeated in the same shape each time:
- feat(agent): expose spilled tool output as an agent-callable read tool #13754 closed on
assert hasattr(ToolRegistry, "read_spilled_output")— which passes for any unwired method. - fix(agent): wire read_spilled_output into the production tool seam — execute_tool has no production callers #13919 (this issue) was then closed citing
tool_registry.pygrep counts — already true when it was filed.ToolRegistry.execute_toolhas no production callers; the live seam is_dispatch_tool_call. - PR fix(agent): offload oversized tool output at the chat seam, where the model actually sees it (#13997) #14237's first attempt put the run bind in
_run_llm_iterations— a wrapper the LangGraph path bypasses entirely. Green tests, dead feature.
Every time the evidence came from the layer that had just been changed. What finally worked was pointing each test at something I had not touched: the seam the other path calls, and the factory that builds the real result.
Attempt 3 also repeated a documented mistake: #11612 had already put the cost badge in that same bypassed wrapper, diagnosed it, fixed it, and left the explanation six lines from where I made the same error.
What is still open on this mechanism
The spill is now genuinely reachable, and it stays off (
AUTOBOT_TOOL_OUTPUT_SPILLdefaults false). Before it is enabled:- fix(chat): an LLC tool result stores a dict under output, so the offload silently skips it whatever the key list says #14284 — an LLC tool result stores a dict under
output, and the adapter guards onisinstance(..., str), so it is skipped whatever the key list says. Fails on type, not name. - fix(chat): tool-output offload does not fire on the approval-gate resume path #14242 — with
AUTOBOT_CHAT_APPROVAL_GATEon,execute_toolsbuilds its own history and bypasses the offload entirely.
Related: #13692 · #13754 · #13865 · #13933 · #13997 / PR #14237 · #13991
-
PR #13918 makes
read_spilled_outputdispatchable throughToolRegistry.execute_tooland verifies it end to end. That is genuine, but it is only half of #13754, and the half it delivers is not the live path.The gap
ToolRegistry.execute_toolhas no production callers. From the registry, production reaches onlyget_available_tools()/get_raw_descriptions()(viaservices/llm_service.py).The live tool seam is
chat_workflow/tool_handler.py::_dispatch_tool_call, which has noread_spilled_outputbranch. An unknown name there lands in_build_unknown_tool_errorand incrementsconsecutive_invalid_tool_calls— so with the spill enabled, an agent following the excerpt's instruction would burn its invalid-call budget.Why it is filed rather than fixed in #13918
Wiring the production seam is a larger change than a defect-fix PR should carry, and with
AUTOBOT_TOOL_OUTPUT_SPILLoff nothing reaches it. Filing it explicitly rather than describing #13754 as delivered — that issue was already closed once on a capability that could not be invoked, and the correction should not repeat the same shape at a different layer.Acceptance criteria
_dispatch_tool_callroutesread_spilled_outputto the registry methodToolRegistry.execute_tool, and fails if the branch is removedconsecutive_invalid_tool_callsread_spilled_windowfails closed without one, so a correctly-wired dispatch with no bound run still returns nothing usefulBlocked by
Nothing. But it is only observable with the spill enabled, so it should land with (or before) any change that turns
AUTOBOT_TOOL_OUTPUT_SPILLon.Related: #13865, #13692, PR #13918.