fix(scratchpad): stop large MCP outputs from producing spurious tool-call errors#263
Open
adrianfurlong wants to merge 2 commits into
Open
fix(scratchpad): stop large MCP outputs from producing spurious tool-call errors#263adrianfurlong wants to merge 2 commits into
adrianfurlong wants to merge 2 commits into
Conversation
The persistence wrapper appends an inline `[Tool output saved to artifact: ...]` footer to large tool outputs. Because ComposedWrapper runs transform_output in reverse order, persistence runs before the scratchpad wrapper, so the scratchpad wrote the footer-bearing output verbatim to disk. For JSON tool results (e.g. Prometheus range queries) this appended a non-JSON line after the closing brace, making the file invalid JSON and breaking get_in/schema/iterate_over with "Not JSON: file is not valid JSON". Strip the footer before counting, hashing, and writing. Passthrough returns keep the original output so the footer still serves as the read_artifact pointer when the scratchpad does not intercept. Stripping before hashing also fixes a dedup gap where identical raw outputs hashed differently due to call-specific footer text.
The LLM frequently passes a literal snippet (e.g. a JSON fragment
like `"metric": {`) as the grep pattern; the unescaped `{` fails to
compile as a regex ("unclosed counted repetition"), surfacing a
noisy Invalid regex tool-call error. Fall back to a literal
(escaped) search so grep behaves like the substring match the model
expected.
Also reword the NotJson error to point the model at
read/head/grep/slice for non-JSON files, so it self-corrects instead
of retrying a JSON-only tool.
c4c279a to
cbd2172
Compare
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.
Summary
Two recoverable-but-noisy
ToolCallErrorwarnings were appearing in production when the agent used MCP tools that return large outputs (e.g. Prometheus range queries):Not JSON: file is not valid JSONInvalid argument: Invalid regex: regex parse error … unclosed counted repetitionBoth originate in the scratchpad layer, not the MCP servers. This PR fixes the root causes.
Root cause & fixes
1. Persistence footer corrupted the scratchpad file (
wrapper.rs)ComposedWrapperrunstransform_outputin reverse order, so the persistence wrapper runs before the scratchpad wrapper and appends an inline[Tool output saved to artifact: …]footer. The scratchpad then wrote that footer-bearing output verbatim to disk. For JSON tool results this left a non-JSON line after the closing brace, so the file was invalid JSON andget_in/schema/iterate_overfailed withNot JSON.Fix: strip the footer before counting, hashing, and writing. Passthrough returns keep the original output, so the footer still serves as the
read_artifactpointer when the scratchpad doesn't intercept. Stripping before hashing also closes a dedup gap (identical raw outputs previously hashed differently due to call-specific footer text).2.
greprejected literal patterns (tools.rs)The model often passes a literal JSON fragment (e.g.
"metric": {) as the grep pattern; the unescaped{fails to compile as a regex.grepnow falls back to a literal (escaped) search instead of erroring.3. Clearer non-JSON error (
tools.rs)The
NotJsonmessage now points the model atread/head/grep/sliceso it self-corrects instead of retrying a JSON-only tool.Tests
New regression tests (all passing;
cargo clippy -p auraclean):test_strip_artifact_footertest_wrapper_strips_persistence_footer_before_writetest_grep_falls_back_to_literal_on_invalid_regexRelated
Not JSONsymptom (double-encoded JSON from MCP). Not fixed here, but the clearer error message helps that case too.