fix(run_agent): skip OpenAI client rebuild for anthropic_messages api_mode

[zhiyanliu]

Fixes #36693.

What

Short-circuits _replace_primary_openai_client to a no-op success
(returns True, logs at info) when agent.api_mode == "anthropic_messages". Covers all rebuild call-sites in one place
(stale_stream_pool_cleanup, <provider>_credential_refresh,
nous_credential_refresh, copilot_credential_refresh,
credential_rotation, dead_connection_cleanup,
fallback_timeout_apply, recreate_closed:*).

Why

On provider=bedrock + Claude, agent.client is intentionally None
— real requests flow through agent._anthropic_client. The rebuild
attempt is dead-code on this path: it constructs OpenAI(**{}), which
fails with the misleading

error=The api_key client option must be set either by passing api_key
to the client or by setting the OPENAI_API_KEY environment variable

The conversation isn't actually broken (the next API request succeeds),
but the warning is confusing for a Bedrock setup. Issue #36693 has the
full trace.

Design choices

• Short-circuit at _replace_primary_openai_client, not
  create_openai_client. The factory's existing per-provider early
  branches (copilot-acp, google-gemini-cli, gemini) all return
  adapter clients; anthropic_messages has no client to return, so the
  semantically clean place to short-circuit is one layer up where the
  caller decides whether a rebuild is needed at all. This also leaves
  the factory's contract ("return a non-None client") intact.
• Log at info, not silent. "Rebuild skipped for anthropic_messages api_mode (%s) %s" keeps the no-op observable in
  agent.log without raising the warning bar.

Tests

tests/run_agent/test_replace_primary_openai_client_anthropic_messages.py
(3 tests, all passing locally):

• test_replace_primary_openai_client_short_circuits_for_anthropic_messages
  — verifies stale_stream_pool_cleanup returns True without invoking
  the factory and agent.client stays None.
• test_replace_primary_openai_client_short_circuits_for_all_rebuild_reasons
  — loops over all 8 rebuild reasons in the codebase and asserts each
  short-circuits.
• test_replace_primary_openai_client_still_rebuilds_for_openai_api_mode
  — sanity check that chat_completions api_mode still goes through
  the factory.

$ pytest tests/run_agent/test_replace_primary_openai_client_anthropic_messages.py -v
========================= 3 passed in 10.63s =========================

Happy to revisit the design choices in review.

[zhiyanliu]

For reviewer context — there is now a parallel PR #36714 that fixes the same issue at a single call-site (the stale-stream watchdog in agent/chat_completion_helpers.py) by branching on api_mode and calling _rebuild_anthropic_client() instead of _replace_primary_openai_client(). That approach actually rebuilds the Anthropic client so the next request gets fresh connections — useful for stale_stream_pool_cleanup and dead_connection_cleanup, where the trigger semantically implies dead sockets.

This PR's no-op short-circuit covers all 8 rebuild call-sites in one place but does not rebuild the Anthropic client on connection-pool-cleanup reasons. For Bedrock + Claude that's probably fine in practice (the next request via _anthropic_client opens a new connection if needed), but it does lose the explicit pool-cleanup signal.

If you'd prefer a strict superset of both directions, I'm happy to update this PR to:

• Short-circuit at the entrypoint (current behavior, covers all 8 reasons)
• Plus call self._anthropic_client.close() + self._rebuild_anthropic_client() when reason in ("stale_stream_pool_cleanup", "dead_connection_cleanup"), so the connection-pool-cleanup intent is preserved on anthropic_messages

Just let me know which shape you'd like and I'll push the update. Either way, happy to close this in favor of #36714 if you'd rather keep the per-call-site pattern.

[mxnstrexgl]

🤖 Automated PR Review

Security Scan

• ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.

Code Quality

• ✓ No blocking code-quality issues found by this automated scan.

Summary

Status: APPROVE — security findings: 0, quality suggestions: 0.

Automated review; raw diff content intentionally omitted.

[teknium1]

Closed in favor of #53926, now merged to main (commit a0b9663).

This was a popular bug — 12 independent PRs fixed the same issue (#28161): the three stream-cleanup paths in the streaming code rebuilt the OpenAI primary client unconditionally, which on Anthropic-native sessions both failed (no OPENAI_API_KEY) and left the wedged stream open, causing the ~15-minute hang.

We salvaged @EloquentBrush0x's #28240 (the earliest dedicated fix for this issue) onto current main, resolved the conflict against the newer request-client cleanup helper, repointed the bug-encoding regression test, and added coverage for the two reachable Anthropic cleanup sites. Verified live: stale stream torn down in <1s instead of ~900s, Anthropic client closed+rebuilt, OpenAI rebuild never called on the Anthropic path.

Thank you for the fix — closing as a duplicate of the merged work. Credit to everyone who reported and fixed this.

#53926