Skip to content

fix(runtime): recover reset streams and deduplicate terminal model errors - #16

Merged
Lei-k merged 1 commit into
mainfrom
fix/issue-15-reset-recovery
Sep 6, 2026
Merged

Lei-k merged 1 commit into
mainfrom
fix/issue-15-reset-recovery

Conversation

@Lei-k

@Lei-k Lei-k commented Sep 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes #15.

  • Derive the primary-recovery transport gate from the classifier so streamed ReadError/reset failures can enter the existing bounded client-rebuild recovery cycle.
  • Give chat final delivery ownership of the reported terminal API failure, rather than emitting the same warning through status and final channels.
  • Distinguish an interrupted response, a refused/unroutable connection, and an ambiguous connection error. A reset no longer asserts that the model server is stopped.
  • Preserve provider/model/credential selection, programmatic status channels, redaction, and retry/fallback guards.

Candidate: 497cc47556b10eba94c65147cceedeba622aa087
Base: 64fc0d989347df727d58f4d29a74c0359978bd4d

Evidence

  • The reported turn had an earlier successful call followed by three ReadError/Errno 104 failures. Attribution remains upstream-or-network-path; no claim that the gateway was stopped or that all future resets are eliminated.
  • The three new regression files fail on the unchanged base: 29 passed / 19 failed.
  • Final focused writer gate: 419 passed, 0 failed across 14 files.
  • A real loopback TCP reset produces httpx.ReadError, reaches client rebuild, and completes a real successful follow-up request without changing key/model/route.
  • Fresh independent review: PASS, P0/P1 = 0. Independent suites: 48 passed; 278 passed; 86 passed. The reviewed eight-file snapshot was hash-matched to this exact candidate.
  • Changed-commit secret scan: 5 generic-key findings, all reviewed synthetic literals in mocked/loopback/redaction tests. Raw RED and triage retained; no repository allowlist was added.

Honest wider-test boundary

Deployment boundary

This PR is against fork main 0.21.0. Production remains on 0.20.6; it has NOT been restarted, upgraded, or patched in place.

A separate minimal 0.20.6 backport was prepared and independently reviewed: 191 focused tests plus 283 and 14 nearby tests passed (2 platform skips). Its isolated candidate image executed successfully with no network or production mounts; the embedded source hashes match the reviewed backport. A minimal authenticated request through that backport also succeeded. None of this claims production deployment or production Telegram end-to-end verification.

Human review/merge policy remains intact. Production rollout requires its own approved safe window and read-back; no automatic merge.

…re, and recover from resets

Reported 2026-09-06: a Telegram turn completed an earlier model call, then
exhausted three attempts with `httpx.ReadError: [Errno 104] Connection reset by
peer`, and the user saw the same "the configured model endpoint is not running"
warning twice.

Three defects, each reproduced on this base:

1. Duplicate notice. `agent/turn_recovery.py`'s terminal paths emit the failure
   envelope through `status_callback` AND return it as `final_response`;
   `_prepare_gateway_status_message` and `_sanitize_gateway_final_response` then
   map both onto the same provider-error reply, so chat surfaces post it twice.
   Chat surfaces now take the provider-failure category from the final response
   only. Programmatic surfaces (local/api/webhook) keep both raw channels, and
   the noise/compression/warning filters are untouched.

2. Unsupported diagnosis. One connection reply covered every connection shape.
   A mid-response reset is not evidence that the endpoint is down — the same
   turn had just been answered by it. The reply table now separates an
   interrupted transfer, a refused/unroutable connect (which keeps the
   endpoint-down wording it was written for, NousResearch#86570), and a cause-flattened
   `APIConnectionError`, which names both possibilities and asserts neither.
   Auth / policy / rate-limit classification and redaction are unchanged.

3. Reset recovery was bypassed. `try_recover_primary_transport` — the one place
   that retires the poisoned httpx pool and rebuilds the client before giving up
   — is gated on a hand-maintained type list that omitted `ReadError`, the shape
   64 of 67 failed attempts actually took. The canonical classifier already
   listed it, so the gate is now derived from `TRANSPORT_ERROR_TYPES` and cannot
   drift again. This changes WHICH failures get the single rebuild, not how many
   attempts anything gets: recovery still fires at most once per API-call block,
   only after the classifier called the error retryable and the retry budget is
   spent. No retry-count, model, credential or TLS changes.

Tests: `tests/run_agent/test_readerror_reset_recovery.py` injects a real TCP RST
from a local listener, so the `httpx.ReadError` driving recovery is produced by
httpx rather than constructed, and the rebuilt client goes on to complete a real
request. The gateway tests drive the real terminal path and assert the delivery
contract (exactly one user-visible bubble) rather than any wording.

This improves local recovery and messaging. It does not, and cannot, remove the
upstream/intermediary resets themselves; the logs cannot attribute those.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

running on 497cc47 — fix(gateway,agent): one accurate notice per terminal connect


Still running 2 jobs: OS-specific tests / Windows-only tests, Python tests / Run tests

⚠️ Warnings

OSV vulnerability scan · View job

28 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.

@Lei-k
Lei-k marked this pull request as ready for review September 6, 2026 08:24
@Lei-k
Lei-k merged commit 5cd65c0 into main Sep 6, 2026
31 of 34 checks passed
teknium1 pushed a commit to NousResearch/hermes-agent that referenced this pull request Sep 20, 2026
Split the single connection row of the gateway's shaped provider-error reply
into three: an interrupted established connection (reset/EOF/RemoteProtocolError),
a refused/unroutable endpoint (the case #86570 wrote the "not running or is
unreachable" wording for), and a cause-free SDK ``APIConnectionError`` that
supports neither diagnosis. A reset says nothing about whether the endpoint is
up, so telling the user to restart a server that just answered sends them to
debug the wrong thing (#116323).

Selectively adapted from Lei-k#16 (497cc47) via PR #109701,
rebased onto the current reply contract (rate-limit > auth > policy > connection,
every reply names a slash command, no operator jargon).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(runtime): recover reset response streams and avoid duplicate terminal error notices

1 participant