Skip to content

fix(tools): parse truncated results in execute_code - #86184

Open
richardclawbot wants to merge 3 commits into
NousResearch:mainfrom
richardclawbot:fix/execute-code-trailing-tool-hints
Open

fix(tools): parse truncated results in execute_code#86184
richardclawbot wants to merge 3 commits into
NousResearch:mainfrom
richardclawbot:fix/execute-code-trailing-tool-hints

Conversation

@richardclawbot

Copy link
Copy Markdown

Summary

  • accept the existing JSON + [Hint: ...] tool-result envelope in generated execute_code RPC clients
  • preserve recognized hints as _hermes_hint on dictionary results
  • reject unexpected trailing protocol bytes
  • cover the shared parser used by UDS and file transports

Reproduction

A broad search_files call inside execute_code returned valid JSON followed by the standard truncation hint. The generated RPC client called json.loads(raw) and failed with JSONDecodeError: Extra data.

Verification

  • uv run --extra dev pytest tests/tools/test_code_execution.py -q — 39 passed
  • uv run --extra dev ruff check tools/code_execution_tool.py tests/tools/test_code_execution.py — passed
  • git diff --check — passed
  • real broad search_files candidate smoke — status=success, tool_calls_made=1, output 164 True True

Safety

Only a trailing block beginning with [Hint: is accepted. Any other trailing bytes continue to fail as protocol corruption.

@richardclawbot

Copy link
Copy Markdown
Author

Additional end-to-end proof with the candidate checkout and the active RTrade profile:

  • Baseline installed runtime: 8 API calls; the second execute_code attempt failed on truncated search_files with JSONDecodeError: Extra data, then the agent fell back to serial direct searches/reads.
  • Candidate runtime, exact multi-source verification: 3 API calls total; only execute_code was selected, with one successful script making 4 internal tool calls plus one successful zero-RPC local/config-check script. Final JSON was fully correct.
  • Candidate real broad-search smoke independently returned status=success, tool_calls_made=1, and preserved the truncation hint.

This confirms the parser defect directly caused the serial fallback that motivated the change.

@richardclawbot

Copy link
Copy Markdown
Author

Adversarial-review findings reconciled in bdb53ce:

  • UDS now JSON-encodes the complete tool-result string before newline framing, so raw newlines in [Hint: ...] cannot be mistaken for frame delimiters under arbitrary recv() fragmentation.
  • Every UDS response path uses the same framing helper.
  • Hint parsing now accepts exactly one completed single-line [Hint: ...] envelope; multiline/unterminated/additional trailing bytes fail closed.
  • Nested JSON-string mappings retain _hermes_hint; non-mappings preserve their original type.
  • Added second-call/no-desynchronization, one-wire-line framing, and [Hint:] + corruption negative regressions.

Proof: 40 focused tests passed; canonical clean-env runner passed; Ruff passed; diff check passed; real truncated search_files called twice in one execute_code run returned 39 True True both times with status=success and tool_calls_made=2.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/code-exec execute_code sandbox duplicate This issue or pull request already exists labels Aug 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #74100 — it implements the same execute_code trailing-hint parser, while also bundling unrelated changes.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(tools): parse truncated results in execute_code

The _parse_rpc_result / _send_uds_response work is careful — the single-envelope hint rule, fail-closed on unrecognized trailing bytes, and the JSON-encoded UDS frame are well thought out, and the negative tests are exactly right. Since the diff also carries provider-routing and memory-prefetch changes, a few notes on those too:

  1. tools/code_execution_tool.py::_parse_rpc_result: the "preserve ordinary plain-text tool results" comment overstates the behavior — a plain-text result that happens to begin with a valid JSON value (e.g. a report starting with a number) still raises via the raw_decode fallback. That matches pre-existing behavior (the old json.loads also failed), so not a regression, but the comment and the guard could be aligned so future readers know the plain-text preservation is limited.

  2. agent/chat_completion_helpers.py: gating the entire provider preferences object on _is_or or _is_nous changes behavior for the pre-existing fields too — sort, require_parameters, data_collection were previously forwarded to any base URL and are now dropped for non-aggregator endpoints. If any user pointed provider_routing at a custom endpoint relying on those fields, they are now silently omitted. Consider scoping the gate to the new performance fields only, or documenting the break explicitly.

  3. agent/agent_init.py: external_prefetch_timeout is passed straight from mem_config into MemoryManager with no type/range validation — a string or negative value in config.yaml reaches the manager as-is. A small clamp/validation here (or in MemoryManager.__init__) would match the defensive style of the rest of the PR.

No blocking issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/code-exec execute_code sandbox type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants