fix(harness): recover truncated/unterminated JSON tool requests - #1798
Open
Zenetusken wants to merge 10 commits into
Open
fix(harness): recover truncated/unterminated JSON tool requests#1798Zenetusken wants to merge 10 commits into
Zenetusken wants to merge 10 commits into
Conversation
DeepSeek V4 Flash intermittently breaks Agent Zero's tool protocol in three distinct ways, all reproduced against the live API and covered by new regression tests (tests/test_deepseek_harness_reliability.py). 1. Planning prose around the JSON tool envelope. With thinking enabled, the model occasionally prepends prose to an otherwise valid tool request. extract_tool_request() stays strict; process_tools() now falls back to recover_embedded_tool_request(), which accepts exactly one valid embedded envelope and rejects zero/ambiguous/multiple. 2. Provider-dropped streams. DeepSeek closes streaming connections mid-response; LiteLLM ends the iterator without an error, so partial unterminated JSON reached the parser as "Message misformat". The transport now tracks last_finish_reason; a DeepSeek chat-completions stream ending without one is discarded and the turn retried (bounded by a0_retry_attempts, early-stop and responses API exempt). 3. Truncated non-streaming bodies classified fatal. LiteLLM reports "Unable to get json response" with the original 200 status code; _is_transient_litellm_error now treats it as transient so utility calls retry instead of surfacing immediately. Also: - Propagate finish_reason through chat-completions parse, stream parser, LLMResult (dict/metadata roundtrip) and the unified_turn fallback, so diagnostics can distinguish length-truncation from dropped streams. - Misformat warnings now carry a sanitized reason (truncation, prose, missing envelope, empty) plus output length, never content. - Serialize the _50/_51 memory post-processing jobs on a shared async lock (concurrent long-lived utility requests were being truncated provider-side) and include the provider status code in their errors.
Two more V4 Flash failure modes, both root-caused from saved chat metadata and covered by regression tests. 1. Empty completions (production seq 1082): thinking/JSON mode occasionally finishes with finish_reason=stop after a full reasoning stream but whitespace-only content. The turn was accepted as valid and reported as a misformat. New _should_retry_empty_completion() discards whitespace-only DeepSeek completions and retries the turn (same bounds and exemptions as the dropped-stream retry). 2. Truncated non-streaming bodies: LiteLLM reports "Unable to get json response" with the original HTTP 200 status, which _is_transient_litellm_error trusted and never retried (the _51_memorize_solutions failure). It is now classified transient regardless of status code.
Root cause: after an Agent Zero restart the in-memory /ws registry is wiped, and the a0 CLI's bounded reconnect (5 attempts over ~38s) often gave up before the server was ready, leaving remote execution dead until manual intervention. Meanwhile code_execution_remote returned a soft tool result, so the model kept retrying with fresh session numbers instead of surfacing the infrastructure problem to the user. - code_execution_remote: when no CLI is connected, wait up to 60s (2s poll) for the CLI's automatic reconnect before failing; on failure end the turn (break_loop) with actionable reconnect guidance instead of inviting endless retries. Same clean handoff when a CLI is connected but exec/write access is disabled. - helpers: add __init__.py so the package is no longer imported as a namespace package. Test cleanup helpers that purge sys.modules treated the namespace package as a stub and deleted every loaded helpers.* module, splitting extension-registry state between stale and fresh module copies depending on import order (e.g. Agent instances losing extension-initialized loop_data). Full suite goes from 39-71 flaky failures to a deterministic 7 pre-existing environmental ones (1203 passed). - tests: regression coverage for the reconnect grace window, the break-loop handoff, and the exec-disabled/write-blocked fast paths.
Full suite goes from 39-71 order-dependent failures plus 6 collection errors to a deterministic 1218 passed / 0 failed. - conftest: bootstrap the Telegram plugin's lazy aiogram install via its own ensure_dependencies() before collection, falling back to collect_ignore_glob for offline environments; exclude the two legacy manual scripts (email_parser_test.py imports a commented-out module, rate_limiter_test.py performs a live LLM call at import). - test_http_auth_csrf: register the serve_index endpoint the auth fallback redirects through (was 500 via werkzeug BuildError instead of 302); assert the login redirect's next parameter semantically instead of pinning Werkzeug's old query encoding. - test_docker_release_plan: track the current branch-promotion contract (testing/ready/main, TARGET_TAG re-resolution) after the workflow refactor left the assertions stale. - test_browser_agent_regressions: install sys.modules stubs only when the real module is unimportable - unconditional collection-time stubs poisoned every later-collected module depending on import order (collateral included test_responses_architecture and the file's own viewer tests); shadow emit_to on the handler instance in the viewer command tests so they behave identically in both regimes.
Findings from three adversarial code reviews of the DeepSeek harness, CLI disconnect, and test-suite repair commits: Arc 1 (DeepSeek harness): - extract_tools: tighten embedded-JSON recovery — mask quoted, fenced, and inline-code regions before scanning, and require strict JSON for candidate objects, so prose examples can no longer be recovered as live tool requests (M1) - models: classify transient failures status-first so a mis-mapped exception cannot trigger retries of non-transient errors (M3) - models: gate empty-completion retries to direct DeepSeek transports only, avoiding double-billing through proxies (M4) - memorize_lock: rebind the loop reference so the lock stays valid if the background event loop is recreated (M5) Arc 2 (CLI disconnect survival): - code_execution_remote: exit the grace wait early when the CLI reconnects, emit progress while waiting, and drop a stale hint (server) - connection.py (host CLI, deployed separately): surface disconnect errors instead of looping silently, close the shutdown race, and guard base_url derivation Arc 3 (test suite): - conftest: narrow the aiogram-install except clause and warn loudly on failure instead of masking import errors - test_http_auth_csrf: assert on origin netloc rather than substring - test_docker_release_plan: expect TARGET_TAG twice and validate ALLOWED_BRANCHES - test_model_config_project_presets: tear down the preset cache so the test cannot leak state Verification: full suite in-container 1230 passed, 0 failed, deterministic across two runs after deploy + container restart.
… usr data Root-caused why a full-suite run wiped the user's DeepSeek model presets, reset every scoped model selection to "Default", and clobbered .env timezone defaults to UTC/0: - tests/test_time_travel.py intentionally used PROJECT_ROOT/usr as scratch space; inside the deployed container that path IS the live persistent volume. Redirect helpers.files._base_dir to tmp_path in the workspace fixture and the symlink-alias test so /a0/usr display paths resolve into tmp. Verified: suite passes with /a0/usr mounted read-only. - tests/conftest.py: add an autouse guard that fails any test writing under the real <repo>/usr via helpers.files write/delete functions or the dotenv/localization save_dotenv_value path. The guard immediately caught test_parallel_tool persisting a poll timezone. - helpers/state_snapshot.py: the /poll snapshot builder applied the request timezone via localization.set_timezone(), persisting a browser-reported (or spoofed, e.g. Firefox RFP) timezone into usr/.env as DEFAULT_USER_TIMEZONE. A read poll must never write .env; now persist=False. - helpers/localization.py + helpers/settings.py: set_timezone gains a persist flag; AUTO timezone resolution no longer overwrites the user's saved default. - tests/test_timezone_regressions.py: regression test asserting AUTO mode does not persist the browser timezone. - tests/test_defer_lifecycle.py: drop the environment-dependent "owner collected" assertion after a suppressed CancelledError - asyncio anchors the exception traceback island based on process-wide import state (importing helpers.settings makes it survive pumping and even gc.collect()), so the assertion tested CPython internals, not the DeferredTask contract. The meaningful guarantees (stored call cleared, running arguments untouched, restart rejected) remain asserted. Verification: full suite 1230 passed with /a0/usr mounted read-only; deployed to the live container, 1231 passed, and diff of /a0/usr before/after the run is completely clean.
- Add is_truncated_tool_request() and _json_root_object_balanced() - Route truncated requests to fw.msg_truncated_request.md and loop guard - Extend DeepSeek reliability/connector regression coverage
- recover_embedded_tool_request: refuse 'response'-tool recovery when the
envelope sits in substantial deliberating prose (>40 non-ws chars outside
the JSON root, or hedging like could/but/option), so hedged deliberation
can no longer silently complete a task; operational tools keep being
recovered (helpers/extract_tools.py)
- process_tools: on successful embedded-envelope recovery add the new
fw.msg_recovered_request.md corrective note so the model learns to emit
bare JSON; deliberately not tracked by the unusable-response-loop guard
(agent.py, prompts/fw.msg_recovered_request.md)
- is_truncated_tool_request: replace the substring keyword gate with a
structural prefix match (thoughts/headline/tool_name/tool_args/type) so
prose mentioning 'actions' after a stray '{' is not swallowed into a
repair loop in responses mode; truncated function_call payloads classify
as truncated so they take the repair-prompt path (helpers/extract_tools.py)
- unified_call/unified_turn: utility callers opt out of the main-turn
empty-completion retry via a0_allow_empty_completion; call_utility_model
passes it because an empty utility reply (e.g. 'nothing to memorize') is
benign (models.py, agent.py)
- dedupe the brace-depth scan shared by _json_root_object_balanced and
is_truncated_tool_request; classify a complete root followed by an
unterminated fragment as trailing truncation in
explain_tool_request_failure; document the '['-prefix recovery blind spot
(helpers/extract_tools.py)
Regression tests cover the review's adversarial inputs, both retry regimes,
and the call_utility_model wiring.
- git rm docs/harness-truncated-json-fix-note.md (internal note with local machine paths and stale status; durable content moves to the PR description) - remove now-unused 'from helpers import errors' from both memorize extensions after the switch to format_utility_error - code_execution_remote: restore the pre-branch guidance that `runtime=output` and `runtime=reset` remain available when writes are blocked
Zenetusken
force-pushed
the
fix/deepseek-harness-reliability
branch
from
August 2, 2026 18:54
f31ce23 to
bec6ea5
Compare
Author
|
Rebased onto v2.8 ( Validation on the rebased branch (v2.8 image runtime): 1306 passed, 3 failed — the 3 failures ( Relationship to v2.8's own envelope repairs: complementary, not overlapping — v2.7/v2.8 repair concatenated envelopes in the native Responses transport, while this PR adds truncation detection ( Note: this branch carries the first version of the |
Zenetusken
added a commit
to Zenetusken/agent-zero
that referenced
this pull request
Aug 2, 2026
…mpt guidance
Live evidence (DeepSeek V4 Flash, 2026-08-04): a fenced 21702-char tool
request cut mid-string logged "truncated or unterminated" via
explain_tool_request_failure() yet routed to the generic misformat
reprompt because is_truncated_tool_request() gated on the raw content
starting with "{". The generic nudge also never addressed payload size,
so the model's retry grew to 31446 chars and truncated again.
- Add classify_tool_request_failure() as the single source of truth for
both the sanitized log reason and the reprompt routing in
Agent.process_tools; explain_tool_request_failure() now derives from it
- Recognize truncation from the first envelope-shaped opening brace, so
fenced, prose-prefixed, and trailing-root truncated payloads classify
correctly (structural envelope gate unchanged)
- fw.msg_truncated_request.md now tells the model to split oversized
payloads into multiple smaller sequential tool calls
- Regression tests for all divergent shapes, category parity with the
log reason, reprompt routing, and nudge content
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.
What this PR does
Hardens the Agent Zero harness against protocol failures observed empirically with the DeepSeek V4 Flash API (saved-chat evidence), without relaxing the strict single-JSON-envelope tool protocol for well-behaved models.
Symptom evidence (from live chat logs)
Message misformat, no valid tool request found.with no diagnostics."tool_args":{"code":"echo) → logged as generic misformat; providerfinish_reason=lengthwas never surfaced, so truncation was indistinguishable from malformed output._51_memorize_solutionsfailing inside LiteLLM with the provider status code swallowed, racing_50_memorize_fragmentson the same background loop.Changes
Parser diagnostics & recovery (
helpers/extract_tools.py,agent.py)finish_reasonis plumbed end-to-end (transport →LLMResult→unified_turn→process_tools) and surfaced in the misformat log reason.explain_tool_request_failure()classifies failures (truncated vs. prose vs. non-tool JSON vs. empty) with sanitized output (class + length only, never response content).recover_embedded_tool_request()recovers exactly one strictly-valid embedded envelope; quoted/fenced/inline-code examples are masked and never executed.is_truncated_tool_request()detects unterminated envelopes via a structural prefix gate ({"thoughts"|"headline"|"tool_name"|"tool_args"|"type") and routes them to a targeted re-prompt (prompts/fw.msg_truncated_request.md) instead of the generic misformat warning; the unusable-response loop guard tracks the new warning so repair loops still terminate.Provider reliability (
models.py)deepseek/providers only (stream, chat API, non-third-party hosts); utility-model calls opt out viaa0_allow_empty_completion(an empty memorize result is a benign outcome — see_50_memorize_fragments.py).Memory post-processing (
plugins/_memory/)_50/_51are serialized via a cancellation-safe async lock (memorize_lock.py) and surface the provider status code in error messages.Adversarial-review hardening (second pass)
response-tool recovery is refused when the envelope sits in substantial deliberating prose (>40 chars outside the root, or hedging words like "could"/"option"/"but") — a task can no longer be "completed" by a possibility under discussion. Successful recoveries now add a corrective history note (prompts/fw.msg_recovered_request.md) teaching the model to emit bare JSON.{here are the actions I plan to takenow classifies correctly and is not swallowed in responses mode.Verification
agent0ai/agent-zero:v2.6image withagent_zero_usrmounted read-only: 1250 passed, 1 skipped (skip = Playwright not installed).Cross-PR note
This branch also contains connector-reconnect, timezone-persistence and test-infrastructure commits that overlap #1799 and #1800. Recommended merge order: #1799 and #1800 first (small, independent), then this PR rebased; or this PR first and the other two shrink correspondingly.