fix(agent): request-local Anthropic clients so the stale/interrupt watchdog never corrupts SQLite (#67142, supersedes #51688) - #67238
Merged
Conversation
…tchdog never corrupts SQLite (#67142) Direct-Anthropic requests used a single shared _anthropic_client, and the stale/interrupt watchdog closed + rebuilt it from the poll (stranger) thread at four sites (non-streaming stale/interrupt, streaming stale/interrupt). Closing a client whose TLS socket a worker thread was still reading released the FD from a stranger thread; the kernel recycled it under a live SSL BIO, which then wrote a 24-byte TLS record into an unrelated SQLite header (cron/executions.db), bricking every cron on the profile. Same shape as the OpenAI-only #29507 fix, but the Anthropic path never got the owner-thread contract. Extend the #29507 ownership contract to Anthropic: build a per-request client (_create_request_anthropic_client), register it with the request-client holder tagged by kind, and route _close_request_client_once by kind — a stranger thread only shuts the request client's sockets down (_abort_request_anthropic_client), while the owning worker performs the SDK close (_close_request_anthropic_client). The shared _anthropic_client is now never closed from inside a request (streaming or non-streaming), including the worker retry-cleanup sites, since each attempt builds a fresh request client. The #28161 no-hang guarantee is preserved: the poll-thread socket abort unblocks the worker immediately. Salvages the approach from #51688 (@raymondyan-zhijie), reimplemented onto current main (non-streaming dispatch was refactored into _dispatch_nonstreaming_api_request; streaming grew _cancel_current_stream_attempt and worker retry-cleanup sites). Tests updated to the request-local mechanism (incl. replacing a banned source-reading test with a behavior test) plus new regression coverage proving the watchdog aborts the request client and never touches the shared client. Co-authored-by: raymondyan-zhijie <32435458+raymondyan-zhijie@users.noreply.github.com>
23 tasks
Collaborator
Related to #67210 and #51688: this salvage uses request-local Anthropic clients across the watchdog paths, while #67210 preserves the shared-client model with owner-thread teardown. Both address #67142 but differ in contract and scope; maintainer selection is needed. |
3 tasks
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…owner-thread-abort fix(agent): request-local Anthropic clients so the stale/interrupt watchdog never corrupts SQLite (NousResearch#67142, supersedes NousResearch#51688)
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
Salvages #51688 (@raymondyan-zhijie) for #67142 — the direct-Anthropic gateway corruption where a stale/interrupt watchdog closed the shared Anthropic client from the poll thread and recycled a live TLS FD into a SQLite header, bricking
cron/executions.db(every due cron then failed atcreate_execution()).Root cause: direct-Anthropic requests share one
_anthropic_client, and four watchdog sites closed + rebuilt it from the poll (stranger) thread:interruptible_api_call)interruptible_streaming_api_call)Closing a client whose TLS socket a worker thread was still reading releases the FD from a stranger thread; the kernel recycles it under a live SSL BIO, which writes a 24-byte TLS record over an unrelated SQLite header. This is the same shape as #29507, but the OpenAI-only owner-thread fix never covered the Anthropic path.
Fix — extend the #29507 ownership contract to Anthropic:
_create_request_anthropic_client/_close_request_anthropic_client/_abort_request_anthropic_clienton the agent (mirror the OpenAI request-client helpers; direct + Bedrock + 1M-beta-drop construction).kind(openai/anthropic_messages);_close_request_client_onceroutes by kind — a stranger thread onlyshutdown()s the request client's sockets, the owning worker performs the SDK close._anthropic_messages_create(..., client=...)and_call_anthropic(request_client)use the per-request client instead of the shared one._anthropic_clientis never closed from inside a request — including the worker retry-cleanup sites, since each attempt builds a fresh request client. The two worker-owned retry cleanups the issue flagged stay worker-owned (they now close the request client).#28161 no-hang preserved: the poll-thread socket abort unblocks the worker immediately; the next attempt builds a fresh client, so no dead shared pool lingers.
Why supersede instead of merging #51688
#51688 had the right approach but predated two refactors on
main:_dispatch_nonstreaming_api_request(make_client=...); the per-request Anthropic client is now threaded through a kind-awaremake_clientthere rather than an inlineif/elif._cancel_current_stream_attemptand worker retry-cleanup sites (Anthropic streaming: stale/retry paths call _replace_primary_openai_client, causing 15-min hang on stuck streams #28161 lineage) that fix(agent): avoid closing shared Anthropic client on interrupt #51688 never saw.Reimplemented onto current
mainand reconciled with those; authorship preserved viaCo-authored-by. (The original commits were authored as "Claude Code" — credited to the human contributor.)Novel changes in the mix
_anthropic_clientis never closed from within a request" — fix(agent): avoid closing shared Anthropic client on interrupt #51688 left the worker retry-cleanup sites closing/rebuilding the shared client; with request-local clients that's redundant churn and still a cross-request close, so it's removed. All request cleanup goes through the request-local client.test_interruptible_rebuilds_anthropic_client, which asserted the now-removed rebuild-on-interrupt behavior viainspect.getsource) with a real behavior test.test_28161/test_streaming/ circuit-breaker tests to the request-local mechanism.Test plan
scripts/run_tests.sh tests/run_agent/ tests/agent/— 8174 pass, 3 fail (pre-existingTestRunOauthSetupTokenfailures, identical on cleanmain, unrelated).test_28161_anthropic_stream_pool_cleanup.py,test_stream_stale_circuit_breaker.py,test_streaming.py,test_cascading_interrupt_6600.py,test_run_agent.py— all green.grepconfirms no_anthropic_client.close()remains in any request path inchat_completion_helpers.py.mcp_servers.<srv>.timeout-style long call; force a stale/interrupt and confirm the ledger stays integrity-clean and the worker unblocks promptly.Fixes #67142