feat(provider): add Anthropic OAuth provider (Claude Pro/Max)#132
feat(provider): add Anthropic OAuth provider (Claude Pro/Max)#132theblazehen wants to merge 7 commits intoMirrowel:devfrom
Conversation
Implements direct Anthropic Messages API integration using OAuth PKCE flow, impersonating Claude Code for Pro/Max subscription access. Includes full OpenAI<->Anthropic format conversion, SSE streaming, mcp_ tool prefixing, token refresh, and live model list fetching from /v1/models. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The scope parameter contains spaces that need to be percent-encoded for a valid URL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I'm beginning my review of the new Anthropic OAuth provider implementation. This is a substantial addition, so I'll be taking a close look at the PKCE flow and format conversion logic. Stand by for my full report! |
![mirrobot-agent[bot]](https://avatars.githubusercontent.com/in/2140342?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall Assessment
This is a very impressive and thorough implementation of the Anthropic OAuth provider. The attention to detail in replicating the Claude Code PKCE flow and handling the nuances of the Messages API (like thinking/reasoning blocks and SSE streaming) is excellent.
The provider is feature-rich, supporting both streaming and non-streaming completions, model list fetching, and automatic token refresh with 401 retry logic.
Architectural Feedback
The architecture fits well with the existing provider structure. The split between AnthropicAuthBase and AnthropicProvider is clean and maintains consistency with other providers in the library.
Key Suggestions
- Resilience: Add a check for the PKCE state file in the setup script to avoid crashes if steps are run out of order.
- Compatibility: Update
datetime.utcnow()to the moderndatetime.now(timezone.utc)for Python 3.12+ compatibility. - Data Integrity: Use
json.dumpsfor tool result content to ensure consistent formatting.
Questions for the Author
- Did you consider yielding the
thinking_signature? It might be useful for downstream consumers who want to verify the thinking process, although I understand it doesn't have a direct OpenAI field.
This review was generated by an AI assistant.
| OAUTH_DIR = Path(__file__).parent / ".." / "oauth_creds" | ||
|
|
||
| async def exchange_code(auth_code: str): | ||
| state = json.loads(STATE_FILE.read_text()) |
There was a problem hiding this comment.
This will raise a FileNotFoundError if Step 2 is run before Step 1. Consider adding a check:
if not STATE_FILE.exists():
print("Error: PKCE state file not found. Please run Step 1 first.")
sys.exit(1)There was a problem hiding this comment.
Good catch, fixed in 7aa20d4. Added the existence check before reading.
| from datetime import datetime, timedelta | ||
|
|
||
| expiry_date = ( | ||
| datetime.utcnow() + timedelta(seconds=expires_in) |
There was a problem hiding this comment.
datetime.utcnow() is deprecated in Python 3.12+. It's recommended to use datetime.now(datetime.timezone.utc) instead. This affects lines 320, 400, and 503 (relative to the diff, these are file lines 200, 280, 383).
There was a problem hiding this comment.
Fixed in 7aa20d4. Both occurrences now use datetime.now(timezone.utc).
| "created": int(time.time()), | ||
| } | ||
|
|
||
| elif delta_type == "signature_delta": |
There was a problem hiding this comment.
The signature_delta is accumulated but not yielded. While not strictly required for OpenAI compatibility, you might want to append it to reasoning_content to preserve the full Anthropic response metadata.
There was a problem hiding this comment.
Intentional — signature_delta is an Anthropic-internal field that doesn't map to anything in the OpenAI streaming format. Accumulating it is just defensive in case we want to log or use it later, but yielding it would confuse OpenAI-compatible clients.
| "Accept": "text/event-stream", | ||
| "anthropic-version": "2023-06-01", | ||
| "anthropic-beta": ANTHROPIC_BETA_FEATURES, | ||
| "user-agent": f"claude-cli/{CLAUDE_CODE_VERSION} (external, cli)", |
There was a problem hiding this comment.
The use of hardcoded User-Agent and beta headers for impersonation is a clever way to access Pro/Max models, but it makes the provider dependent on the Claude Code CLI's internal state. Just a heads-up that this is a potential point of failure if Anthropic tightens their checks.
There was a problem hiding this comment.
Yep, understood. The headers and version strings are designed to be easily updatable in one place (CLAUDE_CODE_VERSION, ANTHROPIC_BETA_FEATURES). If Anthropic tightens checks, updating those constants should be sufficient.
| tool_result = { | ||
| "type": "tool_result", | ||
| "tool_use_id": msg.get("tool_call_id", ""), | ||
| "content": content if isinstance(content, str) else str(content), |
There was a problem hiding this comment.
| "content": content if isinstance(content, str) else str(content), | |
| "content": content if isinstance(content, str) else json.dumps(content), |
json.dumps is safer for complex tool outputs than str().
There was a problem hiding this comment.
Good suggestion, fixed in 7aa20d4. json.dumps() handles nested dicts/lists properly.
- Add state file existence check in setup script before Step 2 - Replace deprecated datetime.utcnow() with datetime.now(timezone.utc) - Use json.dumps() instead of str() for tool result content serialization Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Anthropic now requires a `signature` field on all thinking blocks. OpenAI-format clients (JS SDK etc.) can't preserve signatures across turns, causing 400 errors on multi-turn conversations with thinking. Cache thinking block signatures server-side (keyed by SHA-256 of the thinking content) and re-attach them when converting messages back to Anthropic format. Falls back to dropping thinking blocks gracefully when no cached signature is available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
theblazehen
force-pushed
the
anthropic-provider
branch
from
4b7c709 to
a35b586
Compare
Mirrowel
added
Agent Monitored
Add cache_control ephemeral markers on system prompt, tools, and conversation history so Anthropic caches the prefix across turns (~90% savings on cached input tokens). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Follow-up Review
This is an excellent update. You've addressed all the feedback from the initial review and introduced significant enhancements for conversation continuity and performance.
Assessment of New Changes:
- Thinking Signature Cache: A very clever solution to handle "healing" reasoning content. By caching signatures on the server side, you've ensured that the provider remains compatible with standard OpenAI clients while still being able to resume Claude's thinking process in multi-turn conversations.
- Prompt Caching: The implementation of
_inject_cache_controlis a great optimization that will lead to faster response times and lower costs for users with long conversation histories. - Robustness: The added checks in the setup script and improved tool result serialization are spot on.
Questions for the Author
None
This review was generated by an AI assistant.
|
@greptile |
|
| Filename | Overview |
|---|---|
| src/rotator_library/providers/anthropic_provider.py | Core provider with OpenAI↔Anthropic format conversion and SSE streaming. Contains two logic bugs: Accept: text/event-stream sent to the /v1/models JSON endpoint (may silently prevent live model list fetching), and list-typed tool result content being JSON-serialised instead of passed as native Anthropic content blocks (breaks rich tool outputs with images). Also has a silent swallowing of invalid tool argument JSON and a mutation side-effect in _inject_system_prompt. |
| src/rotator_library/providers/anthropic_auth_base.py | OAuth PKCE authentication base class with token exchange, refresh logic, and credential persistence. Security concern: the PKCE code_verifier is used as the OAuth state parameter, exposing it in the authorization URL (browser history, server logs, referrer headers). Token refresh retry logic and safe credential persistence are implemented correctly. |
| scripts/setup_anthropic_cred.py | Two-step CLI helper for OAuth credential setup. The previously-flagged TOCTOU race (write then chmod) and state file cleanup concerns were raised in prior threads. The core flow is straightforward. |
| src/rotator_library/credential_manager.py | Adds Anthropic to DEFAULT_OAUTH_DIRS and ENV_OAUTH_PROVIDERS. Simple, consistent addition that follows the existing patterns for other providers. |
| src/rotator_library/provider_factory.py | Registers AnthropicAuthBase under the "anthropic" key in PROVIDER_MAP. Minimal, correct change. |
Sequence Diagram
sequenceDiagram
participant U as User/Browser
participant S as setup_anthropic_cred.py
participant A as AnthropicAuthBase
participant AC as claude.ai/oauth
participant T as console.anthropic.com/v1/oauth/token
participant P as AnthropicProvider (proxy)
participant API as api.anthropic.com
Note over S,AC: Step 1 — Generate Auth URL
S->>A: _generate_pkce()
A-->>S: verifier, challenge
S->>A: _build_authorize_url(verifier, challenge)
A-->>S: URL (state=verifier, code_challenge=challenge)
S->>U: Print/open URL in browser
U->>AC: Authorize in browser
AC-->>U: Redirect with code#state
Note over S,T: Step 2 — Exchange Code
U->>S: Paste auth code
S->>A: _exchange_code(code, verifier)
A->>T: POST grant_type=authorization_code
T-->>A: access_token, refresh_token, expires_in
A-->>S: tokens dict
S->>S: Write anthropic_oauth_N.json (chmod 0o600)
Note over P,API: Runtime — Proxied Request
P->>A: get_access_token(credential_path)
A->>A: _is_token_expired(creds)?
alt Token expired
A->>T: POST grant_type=refresh_token
T-->>A: new access_token
A->>A: _save_credentials()
end
A-->>P: access_token
P->>P: _build_anthropic_payload(openai_kwargs)
P->>API: POST /v1/messages (SSE stream)
API-->>P: Anthropic SSE events
P->>P: _anthropic_event_to_openai_chunks()
P-->>P: Yield litellm.ModelResponse chunks
Last reviewed commit: c1dba97
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| result[1]["text"] = ( | ||
| CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"] | ||
| ) | ||
| result.pop(0) |
There was a problem hiding this comment.
accessing result[1]["text"] assumes first system block is type "text", will throw KeyError if it's an image or other block type
| result[1]["text"] = ( | |
| CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"] | |
| ) | |
| result.pop(0) | |
| if len(result) >= 2 and result[1].get("type") == "text": | |
| result[1]["text"] = ( | |
| CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"] | |
| ) | |
| result.pop(0) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/rotator_library/providers/anthropic_provider.py
Line: 363-366
Comment:
accessing `result[1]["text"]` assumes first system block is type "text", will throw KeyError if it's an image or other block type
```suggestion
if len(result) >= 2 and result[1].get("type") == "text":
result[1]["text"] = (
CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"]
)
result.pop(0)
```
How can I resolve this? If you propose a fix, please make it concise.| async def exchange_code(auth_code: str): | ||
| if not STATE_FILE.exists(): | ||
| print("Error: PKCE state file not found. Please run Step 1 first.") | ||
| sys.exit(1) | ||
| state = json.loads(STATE_FILE.read_text()) | ||
| verifier = state["verifier"] | ||
|
|
||
| auth = AnthropicAuthBase() | ||
| tokens = await auth._exchange_code(auth_code.strip(), verifier) | ||
|
|
||
| import time | ||
| creds = { | ||
| **tokens, | ||
| "email": "anthropic-oauth-user", | ||
| "_proxy_metadata": { | ||
| "email": "anthropic-oauth-user", | ||
| "last_check_timestamp": time.time(), | ||
| "credential_type": "oauth", | ||
| }, | ||
| } | ||
|
|
||
| oauth_dir = OAUTH_DIR.resolve() | ||
| oauth_dir.mkdir(parents=True, exist_ok=True) | ||
| existing = sorted(oauth_dir.glob("anthropic_oauth_*.json")) | ||
| next_num = len(existing) + 1 | ||
| file_path = oauth_dir / f"anthropic_oauth_{next_num}.json" | ||
|
|
||
| file_path.write_text(json.dumps(creds, indent=2)) | ||
| os.chmod(file_path, 0o600) | ||
| STATE_FILE.unlink(missing_ok=True) | ||
|
|
||
| print(f"Credential saved to: {file_path}") | ||
| print(f"Access token prefix: {tokens['access_token'][:20]}...") |
There was a problem hiding this comment.
if token exchange fails (line 33), STATE_FILE with PKCE verifier remains on disk; wrap in try-finally to ensure cleanup
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/setup_anthropic_cred.py
Line: 25-57
Comment:
if token exchange fails (line 33), STATE_FILE with PKCE verifier remains on disk; wrap in try-finally to ensure cleanup
How can I resolve this? If you propose a fix, please make it concise.…ing budget - Remove trailing 'Z' from isoformat() output that already includes +00:00 from timezone.utc, which caused _is_token_expired() to always return True (constant refresh churn) - Clamp reasoning budget_tokens to max_tokens - 1 in all code paths (Anthropic requires budget_tokens < max_tokens)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds end-to-end Anthropic support: an interactive PKCE OAuth setup script, a new AnthropicAuthBase for credential management/refresh, an AnthropicProvider for request/stream translation, and integration points in provider factory and credential manager. Changes
Sequence DiagramssequenceDiagram
actor User
participant Script as setup_anthropic_cred.py
participant State as .anthropic_pkce_state.json
participant Browser as Browser
participant OAuth as Anthropic OAuth
participant FS as Filesystem
User->>Script: Run step 1 (no args)
Script->>Script: Generate PKCE verifier & challenge
Script->>Script: Build authorize URL
Script->>State: Save PKCE state
Script->>User: Print URL & instructions
User->>Browser: Open authorize URL
Browser->>OAuth: Request authorization
User->>OAuth: Authenticate & approve
OAuth->>Browser: Return auth code
User->>Script: Run step 2 (with code)
Script->>State: Load PKCE verifier
Script->>OAuth: Exchange code + PKCE verifier
OAuth->>Script: Return access/refresh tokens
Script->>FS: Store credentials file
Script->>State: Delete state file
Script->>User: Print saved path & token snippet
sequenceDiagram
participant Client as Client
participant Provider as AnthropicProvider
participant Auth as AnthropicAuthBase
participant Cache as Token Cache
participant API as Anthropic API
Client->>Provider: acompletion(messages, tools, ...)
Provider->>Provider: Convert OpenAI -> Anthropic payload
Provider->>Auth: get_access_token(credential_id)
Auth->>Cache: Check cached token & expiry
alt Token expired
Auth->>API: POST /token (refresh_token)
API->>Auth: Return new tokens
Auth->>Cache: Update cached tokens
end
Auth->>Provider: Return access_token
Provider->>API: POST /messages (stream=true)
loop Streaming events
API->>Provider: SSE event (content/thinking/tool_use)
Provider->>Provider: Convert to OpenAI-like chunks
Provider->>Client: Yield chunk
end
Provider->>Client: Return final aggregated response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Addressed the review feedback in bae616f: Re: Re: Additionally fixed: expiry timestamp bug — Neither reviewer caught this one. The code was doing |
| def step1(): | ||
| verifier, challenge = _generate_pkce() | ||
| url = _build_authorize_url(verifier, challenge) | ||
| STATE_FILE.write_text(json.dumps({"verifier": verifier, "challenge": challenge})) |
There was a problem hiding this comment.
PKCE state file written world-readable
STATE_FILE.write_text(...) creates the file with the process's default umask (typically 0o644), so any local user can read the PKCE verifier until Step 2 completes. The verifier must remain secret — if an attacker can read it and also intercepts the auth code (e.g. from a shared browser history or redirect log), they can redeem the code themselves. The credential file in exchange_code() is separately protected with os.chmod(…, 0o600), but the state file has no such protection.
| STATE_FILE.write_text(json.dumps({"verifier": verifier, "challenge": challenge})) | |
| STATE_FILE.write_text(json.dumps({"verifier": verifier, "challenge": challenge})) | |
| os.chmod(STATE_FILE, 0o600) |
| file_path.write_text(json.dumps(creds, indent=2)) | ||
| os.chmod(file_path, 0o600) |
There was a problem hiding this comment.
OAuth token file briefly world-readable (TOCTOU race)
file_path.write_text(...) creates the file with default umask permissions (0o644 on most systems), and only the subsequent os.chmod(file_path, 0o600) restricts access. Between those two calls the file containing the access and refresh tokens is readable by any local user.
anthropic_auth_base.py's setup_credential() method handles this correctly by calling safe_write_json(..., secure_permissions=True), which writes atomically with restricted permissions. This script should do the same:
from rotator_library.utils.resilient_io import safe_write_json
import logging
_log = logging.getLogger(__name__)
# replace lines 52-53 with:
safe_write_json(str(file_path), creds, _log, secure_permissions=True)When a client pins a specific tool via tool_choice, the name was inserted unprefixed while all tool definitions carry the mcp_ prefix, causing an invalid_request_error from Anthropic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup_anthropic_cred.py`:
- Around line 52-53: The current code writes credentials with
file_path.write_text(...) and then calls os.chmod(...), which is racy with the
process umask; replace that pattern by calling the existing safe_write_json
helper used by AnthropicAuthBase._save_credentials and pass
secure_permissions=True so the file is created atomically with correct 0o600
permissions from creation time; remove the write_text and os.chmod calls and use
safe_write_json(creds, file_path, secure_permissions=True) (matching the
behavior and API used in AnthropicAuthBase._save_credentials).
- Around line 48-50: The current logic uses len(existing) to pick the next file
number and can collide if earlier files were removed; change the allocation to
scan existing filenames (from oauth_dir.glob("anthropic_oauth_*.json")) extract
the numeric suffixes (e.g. parse the part after "anthropic_oauth_" from each
Path.stem), compute next_num as max(found_numbers, default=0) + 1, and then
build file_path = oauth_dir / f"anthropic_oauth_{next_num}.json" so it always
picks the next unused numeric suffix and never overwrites an existing file;
reference variables: existing, next_num, file_path, oauth_dir.
In `@src/rotator_library/providers/anthropic_auth_base.py`:
- Around line 445-458: initialize_token currently discards in-memory credentials
when creds_or_path is a dict by always calling await self._load_credentials(path
or ""), which loads from an empty path or env; change the logic in
initialize_token to detect dict creds_or_path and use it directly (e.g., assign
creds = creds_or_path) instead of calling _load_credentials, then continue to
check self._is_token_expired(creds) and call _refresh_token only when needed;
keep existing behavior for string paths (call await
self._load_credentials(path)) and ensure display_name logic remains unchanged.
- Around line 481-484: The code currently uses Path(base_dir) when base_dir is
provided, bypassing the library's normalization; change the logic in
anthropic_auth_base (where get_oauth_dir is imported and oauth_dir is computed)
to call get_oauth_dir(base_dir) (instead of Path(base_dir)) so the resolved path
matches what CredentialManager.discover_and_prepare() expects, then ensure
oauth_dir.mkdir(parents=True, exist_ok=True) is still called to create the
directory.
In `@src/rotator_library/providers/anthropic_provider.py`:
- Around line 695-708: Compute the budget using _reasoning_effort_to_budget but
validate it before setting payload["thinking"]: call
self._reasoning_effort_to_budget(reasoning_effort, max_tokens) (with max_tokens
= kwargs.get("max_tokens", 16384)), coerce/ensure it is an integer and then only
add payload["thinking"] when budget > 0 and budget < max_tokens; if budget is <=
0 or >= max_tokens (including cases where max_tokens <= 1) omit
payload["thinking"] entirely so impossible values (0, -1, or >= max_tokens) are
not serialized. Use the existing symbols reasoning_effort,
_reasoning_effort_to_budget, payload["thinking"], and max_tokens to locate and
implement this validation.
- Around line 326-344: The _prefix_tool_names helper currently prefixes tool
definitions and tool_use content blocks but misses forced tool choices; update
_prefix_tool_names to also rename any tool_choice entries so forced selections
match the prefixed definitions: when iterating payload["messages"] and their
content blocks (and also if a message has a top-level "tool_choice" key), detect
dicts with "type" == "tool_choice" or keys named "tool_choice" and, if they
contain a "name" field, replace it with f"{TOOL_PREFIX}{name}"; ensure you keep
the existing deepcopy and other behavior in _prefix_tool_names so all
occurrences of tool names are consistently prefixed (refer to
_prefix_tool_names, TOOL_PREFIX, and tool_choice/tool_use keys to locate
changes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a580e0c0-5e4f-49dd-8c50-367661b1be68
📒 Files selected for processing (5)
scripts/setup_anthropic_cred.pysrc/rotator_library/credential_manager.pysrc/rotator_library/provider_factory.pysrc/rotator_library/providers/anthropic_auth_base.pysrc/rotator_library/providers/anthropic_provider.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (4)
src/rotator_library/provider_factory.py (1)
src/rotator_library/providers/anthropic_auth_base.py (1)
AnthropicAuthBase(75-572)
scripts/setup_anthropic_cred.py (1)
src/rotator_library/providers/anthropic_auth_base.py (3)
_generate_pkce(50-55)_build_authorize_url(58-72)_exchange_code(241-289)
src/rotator_library/providers/anthropic_provider.py (3)
src/rotator_library/providers/provider_interface.py (3)
has_custom_logic(258-263)get_models(244-255)acompletion(265-273)src/rotator_library/timeout_config.py (2)
TimeoutConfig(23-105)streaming(76-89)src/rotator_library/transaction_logger.py (3)
ProviderLogger(472-582)log_error(539-547)log_response_chunk(521-528)
src/rotator_library/providers/anthropic_auth_base.py (3)
src/rotator_library/utils/resilient_io.py (1)
safe_write_json(523-626)src/rotator_library/error_handler.py (1)
CredentialNeedsReauthError(137-159)src/rotator_library/utils/paths.py (1)
get_oauth_dir(74-87)
🪛 Ruff (0.15.4)
scripts/setup_anthropic_cred.py
[error] 65-65: f-string without any placeholders
Remove extraneous f prefix
(F541)
src/rotator_library/providers/anthropic_provider.py
[warning] 13-13: Import from collections.abc instead: AsyncGenerator
Import from collections.abc
(UP035)
[warning] 13-13: typing.List is deprecated, use list instead
(UP035)
[warning] 13-13: typing.Dict is deprecated, use dict instead
(UP035)
[error] 37-44: Consider f-string instead of string join
Replace with f-string
(FLY002)
[warning] 73-73: Missing return type annotation for private function _get_thinking_cache
(ANN202)
[warning] 92-92: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
[warning] 115-115: Do not catch blind exception: Exception
(BLE001)
[warning] 116-116: Logging statement uses f-string
(G004)
[warning] 301-301: Consider moving this statement to an else block
(TRY300)
[warning] 419-419: Missing return type annotation for private function _anthropic_event_to_openai_chunks
(ANN202)
[warning] 714-714: Dynamically typed expressions (typing.Any) are disallowed in effort
(ANN401)
[warning] 735-735: Missing type annotation for **kwargs
(ANN003)
[warning] 736-736: Use X | Y for type annotations
Convert to X | Y
(UP007)
[warning] 742-742: Missing return type annotation for private function make_request
(ANN202)
[warning] 758-758: Missing return type annotation for private function stream_handler
(ANN202)
[warning] 781-786: Abstract raise to an inner function
(TRY301)
[warning] 781-786: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 792-796: Abstract raise to an inner function
(TRY301)
[warning] 829-829: Logging .exception(...) should be used instead of .error(..., exc_info=True)
(G201)
[warning] 829-829: Logging statement uses f-string
(G004)
[warning] 832-832: Missing return type annotation for private function logging_stream_wrapper
(ANN202)
[warning] 847-847: Missing return type annotation for private function non_stream
(ANN202)
[warning] 857-857: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 931-944: Unnecessary dict kwargs
Remove unnecessary kwargs
(PIE804)
src/rotator_library/providers/anthropic_auth_base.py
[warning] 16-16: typing.Dict is deprecated, use dict instead
(UP035)
[warning] 16-16: typing.Tuple is deprecated, use tuple instead
(UP035)
[error] 30-30: Possible hardcoded password assigned to: "ANTHROPIC_TOKEN_ENDPOINT"
(S105)
[warning] 82-82: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
[warning] 119-119: Logging statement uses f-string
(G004)
[warning] 139-139: Async functions should not open files with blocking methods like open
(ASYNC230)
[warning] 139-139: Unnecessary mode argument
Remove mode argument
(UP015)
[warning] 142-142: Consider moving this statement to an else block
(TRY300)
[warning] 144-144: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 144-144: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 144-144: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 145-145: Do not catch blind exception: Exception
(BLE001)
[warning] 146-148: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 146-146: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 146-148: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 165-165: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 165-167: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 171-171: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 187-187: Logging statement uses f-string
(G004)
[warning] 234-234: Logging statement uses f-string
(G004)
[warning] 266-268: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 273-273: Avoid specifying long messages outside the exception class
(TRY003)
[error] 291-291: Boolean-typed positional argument in function definition
(FBT001)
[error] 291-291: Boolean default positional argument in function definition
(FBT002)
[warning] 303-303: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 306-306: Logging statement uses f-string
(G004)
[warning] 339-342: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[error] 341-341: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 374-374: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 396-396: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 396-398: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 409-409: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 427-427: Replace aliased errors with OSError
Replace IOError with builtin OSError
(UP024)
[warning] 433-433: Do not catch blind exception: Exception
(BLE001)
[warning] 434-434: Logging statement uses f-string
(G004)
[warning] 438-438: Use X | Y for type annotations
Convert to X | Y
(UP007)
[error] 439-439: Boolean-typed positional argument in function definition
(FBT001)
[error] 439-439: Boolean default positional argument in function definition
(FBT002)
[warning] 439-439: Unused method argument: force_interactive
(ARG002)
[warning] 454-454: Logging statement uses f-string
(G004)
[warning] 460-460: Logging statement uses f-string
(G004)
[warning] 468-468: Do not catch blind exception: Exception
(BLE001)
[warning] 469-471: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 470-470: Logging statement uses f-string
(G004)
[warning] 508-508: Do not catch blind exception: Exception
(BLE001)
[warning] 521-521: Do not catch blind exception: Exception
(BLE001)
[warning] 546-546: Async functions should not open files with blocking methods like open
(ASYNC230)
[error] 552-553: try-except-continue detected, consider logging the exception
(S112)
[warning] 552-552: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (2)
src/rotator_library/provider_factory.py (1)
10-17: LGTM.The new provider key is wired into the existing auth-class registry without changing factory behavior.
src/rotator_library/credential_manager.py (1)
16-32: LGTM.These Anthropic entries match the new
ANTHROPIC_OAUTH[_N]_*naming and let the provider participate in the existing credential discovery flow cleanly.
| existing = sorted(oauth_dir.glob("anthropic_oauth_*.json")) | ||
| next_num = len(existing) + 1 | ||
| file_path = oauth_dir / f"anthropic_oauth_{next_num}.json" |
There was a problem hiding this comment.
Avoid len(existing) + 1 for the next credential number.
If anthropic_oauth_2.json is missing but anthropic_oauth_3.json still exists, Line 49 still computes 3 and overwrites the existing credential on the next auth flow.
Suggested fix
- existing = sorted(oauth_dir.glob("anthropic_oauth_*.json"))
- next_num = len(existing) + 1
+ existing_nums = [
+ int(path.stem.split("_")[-1])
+ for path in oauth_dir.glob("anthropic_oauth_*.json")
+ if path.stem.split("_")[-1].isdigit()
+ ]
+ next_num = max(existing_nums, default=0) + 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/setup_anthropic_cred.py` around lines 48 - 50, The current logic uses
len(existing) to pick the next file number and can collide if earlier files were
removed; change the allocation to scan existing filenames (from
oauth_dir.glob("anthropic_oauth_*.json")) extract the numeric suffixes (e.g.
parse the part after "anthropic_oauth_" from each Path.stem), compute next_num
as max(found_numbers, default=0) + 1, and then build file_path = oauth_dir /
f"anthropic_oauth_{next_num}.json" so it always picks the next unused numeric
suffix and never overwrites an existing file; reference variables: existing,
next_num, file_path, oauth_dir.
| file_path.write_text(json.dumps(creds, indent=2)) | ||
| os.chmod(file_path, 0o600) |
There was a problem hiding this comment.
Create the credential file with secure permissions from creation time.
Line 52 creates the JSON under the process umask, and Line 53 narrows it only afterward. For access/refresh tokens, this should use the same atomic safe_write_json(..., secure_permissions=True) path already used in AnthropicAuthBase._save_credentials().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/setup_anthropic_cred.py` around lines 52 - 53, The current code
writes credentials with file_path.write_text(...) and then calls os.chmod(...),
which is racy with the process umask; replace that pattern by calling the
existing safe_write_json helper used by AnthropicAuthBase._save_credentials and
pass secure_permissions=True so the file is created atomically with correct
0o600 permissions from creation time; remove the write_text and os.chmod calls
and use safe_write_json(creds, file_path, secure_permissions=True) (matching the
behavior and API used in AnthropicAuthBase._save_credentials).
| path = creds_or_path if isinstance(creds_or_path, str) else None | ||
|
|
||
| if isinstance(creds_or_path, dict): | ||
| display_name = creds_or_path.get("_proxy_metadata", {}).get( | ||
| "display_name", "in-memory object" | ||
| ) | ||
| else: | ||
| display_name = Path(path).name if path else "in-memory object" | ||
|
|
||
| lib_logger.debug(f"Initializing Anthropic token for '{display_name}'...") | ||
|
|
||
| creds = await self._load_credentials(path or "") | ||
| if self._is_token_expired(creds): | ||
| creds = await self._refresh_token(path or "") |
There was a problem hiding this comment.
initialize_token() ignores in-memory credentials.
When creds_or_path is a dict, Lines 445-458 discard it and call _load_credentials(""). That either raises on the empty path or initializes an unrelated env credential if ANTHROPIC_OAUTH_* is set.
Suggested fix
- creds = await self._load_credentials(path or "")
+ if isinstance(creds_or_path, dict):
+ creds = creds_or_path
+ else:
+ creds = await self._load_credentials(path)
if self._is_token_expired(creds):
- creds = await self._refresh_token(path or "")
+ if not path:
+ raise ValueError(
+ "Expired in-memory Anthropic credentials cannot be refreshed without a backing path."
+ )
+ creds = await self._refresh_token(path)🧰 Tools
🪛 Ruff (0.15.4)
[warning] 454-454: Logging statement uses f-string
(G004)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_auth_base.py` around lines 445 - 458,
initialize_token currently discards in-memory credentials when creds_or_path is
a dict by always calling await self._load_credentials(path or ""), which loads
from an empty path or env; change the logic in initialize_token to detect dict
creds_or_path and use it directly (e.g., assign creds = creds_or_path) instead
of calling _load_credentials, then continue to check
self._is_token_expired(creds) and call _refresh_token only when needed; keep
existing behavior for string paths (call await self._load_credentials(path)) and
ensure display_name logic remains unchanged.
| from ..utils.paths import get_oauth_dir | ||
|
|
||
| oauth_dir = Path(base_dir) if base_dir else get_oauth_dir() | ||
| oauth_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
Resolve base_dir through get_oauth_dir(base_dir) here too.
get_oauth_dir(root) normalizes the library's ${root}/oauth_creds location, but Lines 483-484 treat base_dir as the final credential directory. If callers pass the application root, the new Anthropic file lands outside the folder that CredentialManager.discover_and_prepare() scans.
Suggested fix
- oauth_dir = Path(base_dir) if base_dir else get_oauth_dir()
- oauth_dir.mkdir(parents=True, exist_ok=True)
+ oauth_dir = get_oauth_dir(base_dir)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from ..utils.paths import get_oauth_dir | |
| oauth_dir = Path(base_dir) if base_dir else get_oauth_dir() | |
| oauth_dir.mkdir(parents=True, exist_ok=True) | |
| from ..utils.paths import get_oauth_dir | |
| oauth_dir = get_oauth_dir(base_dir) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_auth_base.py` around lines 481 - 484,
The code currently uses Path(base_dir) when base_dir is provided, bypassing the
library's normalization; change the logic in anthropic_auth_base (where
get_oauth_dir is imported and oauth_dir is computed) to call
get_oauth_dir(base_dir) (instead of Path(base_dir)) so the resolved path matches
what CredentialManager.discover_and_prepare() expects, then ensure
oauth_dir.mkdir(parents=True, exist_ok=True) is still called to create the
directory.
| def _prefix_tool_names(self, payload: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Add mcp_ prefix to tool names in definitions and messages.""" | ||
| payload = copy.deepcopy(payload) | ||
|
|
||
| if payload.get("tools"): | ||
| for tool in payload["tools"]: | ||
| if tool.get("name"): | ||
| tool["name"] = f"{TOOL_PREFIX}{tool['name']}" | ||
|
|
||
| if payload.get("messages"): | ||
| for msg in payload["messages"]: | ||
| content = msg.get("content") | ||
| if isinstance(content, list): | ||
| for block in content: | ||
| if isinstance(block, dict) and block.get("type") == "tool_use": | ||
| if block.get("name"): | ||
| block["name"] = f"{TOOL_PREFIX}{block['name']}" | ||
|
|
||
| return payload |
There was a problem hiding this comment.
Prefix forced tool_choice.name too.
This helper renames tool definitions to mcp_*, but a payload built with tool_choice={"type": "tool", "name": "foo"} still keeps the unprefixed name. The final request can therefore advertise mcp_foo while forcing foo, which breaks forced-tool calls.
Suggested fix
if payload.get("messages"):
for msg in payload["messages"]:
content = msg.get("content")
if isinstance(content, list):
for block in content:
if isinstance(block, dict) and block.get("type") == "tool_use":
if block.get("name"):
block["name"] = f"{TOOL_PREFIX}{block['name']}"
+
+ tool_choice = payload.get("tool_choice")
+ if (
+ isinstance(tool_choice, dict)
+ and tool_choice.get("type") == "tool"
+ and tool_choice.get("name")
+ ):
+ tool_choice["name"] = f"{TOOL_PREFIX}{tool_choice['name']}"
return payload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 326 - 344,
The _prefix_tool_names helper currently prefixes tool definitions and tool_use
content blocks but misses forced tool choices; update _prefix_tool_names to also
rename any tool_choice entries so forced selections match the prefixed
definitions: when iterating payload["messages"] and their content blocks (and
also if a message has a top-level "tool_choice" key), detect dicts with "type"
== "tool_choice" or keys named "tool_choice" and, if they contain a "name"
field, replace it with f"{TOOL_PREFIX}{name}"; ensure you keep the existing
deepcopy and other behavior in _prefix_tool_names so all occurrences of tool
names are consistently prefixed (refer to _prefix_tool_names, TOOL_PREFIX, and
tool_choice/tool_use keys to locate changes).
| reasoning_effort = kwargs.get("reasoning_effort") | ||
| if reasoning_effort and str(reasoning_effort).lower() not in ( | ||
| "none", | ||
| "disabled", | ||
| "off", | ||
| "false", | ||
| "disable", | ||
| ): | ||
| payload["thinking"] = { | ||
| "type": "enabled", | ||
| "budget_tokens": self._reasoning_effort_to_budget( | ||
| reasoning_effort, kwargs.get("max_tokens", 16384) | ||
| ), | ||
| } |
There was a problem hiding this comment.
Don't send impossible thinking.budget_tokens values.
For max_tokens == 1, Line 732 returns 1, which already violates this helper's own < max_tokens contract. Numeric values like 0 and -1 also flow through unchanged. The caller should omit thinking when no valid budget exists instead of serializing an impossible budget.
Suggested fix
- payload["thinking"] = {
- "type": "enabled",
- "budget_tokens": self._reasoning_effort_to_budget(
- reasoning_effort, kwargs.get("max_tokens", 16384)
- ),
- }
+ budget_tokens = self._reasoning_effort_to_budget(
+ reasoning_effort, kwargs.get("max_tokens", 16384)
+ )
+ if budget_tokens > 0:
+ payload["thinking"] = {
+ "type": "enabled",
+ "budget_tokens": budget_tokens,
+ }
@@
- return min(budget, max_tokens - 1) if max_tokens > 1 else 1
+ if max_tokens <= 1:
+ return 0
+ return max(1, min(budget, max_tokens - 1))Also applies to: 714-732
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 695 - 708,
Compute the budget using _reasoning_effort_to_budget but validate it before
setting payload["thinking"]: call
self._reasoning_effort_to_budget(reasoning_effort, max_tokens) (with max_tokens
= kwargs.get("max_tokens", 16384)), coerce/ensure it is an integer and then only
add payload["thinking"] when budget > 0 and budget < max_tokens; if budget is <=
0 or >= max_tokens (including cases where max_tokens <= 1) omit
payload["thinking"] entirely so impossible values (0, -1, or >= max_tokens) are
not serialized. Use the existing symbols reasoning_effort,
_reasoning_effort_to_budget, payload["thinking"], and max_tokens to locate and
implement this validation.
| async def get_models(self, api_key: str, client: httpx.AsyncClient) -> List[str]: | ||
| """Fetch models live from Anthropic API, falling back to hardcoded list.""" | ||
| try: | ||
| access_token = await self.get_access_token(api_key) | ||
| headers = self._build_anthropic_headers(access_token) | ||
| resp = await client.get( | ||
| f"{ANTHROPIC_API_BASE}/v1/models", | ||
| headers=headers, | ||
| timeout=10.0, | ||
| ) | ||
| if resp.status_code == 200: | ||
| data = resp.json() | ||
| models = [ | ||
| f"anthropic/{m['id']}" for m in data.get("data", []) if m.get("id") | ||
| ] | ||
| if models: | ||
| return models | ||
| except Exception as e: | ||
| lib_logger.debug(f"Failed to fetch Anthropic models live: {e}") | ||
|
|
||
| return [f"anthropic/{m}" for m in FALLBACK_MODELS] |
There was a problem hiding this comment.
Accept: text/event-stream header on a JSON endpoint
_build_anthropic_headers() always sets "Accept": "text/event-stream", which is correct for the /v1/messages streaming endpoint. However, get_models passes those same headers to GET /v1/models, which is a plain REST endpoint that only returns application/json.
If Anthropic's API honours the Accept header and returns 406 Not Acceptable, resp.status_code will not be 200, causing a silent fallback to the hardcoded FALLBACK_MODELS list on every call — the live model list would never be successfully fetched.
Consider building a separate, minimal header set for non-streaming calls, or at minimum override Accept for this request:
model_headers = {**self._build_anthropic_headers(access_token), "Accept": "application/json"}
resp = await client.get(
f"{ANTHROPIC_API_BASE}/v1/models",
headers=model_headers,
timeout=10.0,
)| if role == "tool": | ||
| tool_result = { | ||
| "type": "tool_result", | ||
| "tool_use_id": msg.get("tool_call_id", ""), | ||
| "content": content | ||
| if isinstance(content, str) | ||
| else json.dumps(content), | ||
| } | ||
| if anthropic_messages and anthropic_messages[-1]["role"] == "user": | ||
| if isinstance(anthropic_messages[-1]["content"], list): | ||
| anthropic_messages[-1]["content"].append(tool_result) | ||
| else: | ||
| anthropic_messages[-1]["content"] = [ | ||
| { | ||
| "type": "text", | ||
| "text": anthropic_messages[-1]["content"], | ||
| }, | ||
| tool_result, | ||
| ] | ||
| else: | ||
| anthropic_messages.append( | ||
| {"role": "user", "content": [tool_result]} | ||
| ) |
There was a problem hiding this comment.
List-typed tool result content is JSON-serialised instead of forwarded as content blocks
When an OpenAI tool-role message carries a content that is already a list of blocks (e.g., a rich tool result containing text + an image), the code serialises the entire list to a JSON string:
"content": content if isinstance(content, str) else json.dumps(content),The Anthropic Messages API accepts content in tool_result blocks as either a plain string or a List[ContentBlock]. By always converting a list to a JSON string, any base64 image blocks or other structured data returned by a tool are sent to the API as raw JSON text rather than as actual image/content blocks — meaning Anthropic cannot decode or render them.
The fix is to pass a list directly when content is already a list:
"content": (
content
if isinstance(content, (str, list))
else json.dumps(content)
),| def _build_authorize_url(verifier: str, challenge: str) -> str: | ||
| """Build the authorization URL for Claude Pro/Max OAuth.""" | ||
| params = { | ||
| "code": "true", | ||
| "client_id": ANTHROPIC_CLIENT_ID, | ||
| "response_type": "code", | ||
| "redirect_uri": ANTHROPIC_REDIRECT_URI, | ||
| "scope": ANTHROPIC_SCOPES, | ||
| "code_challenge": challenge, | ||
| "code_challenge_method": "S256", | ||
| "state": verifier, | ||
| } | ||
| from urllib.parse import urlencode | ||
|
|
||
| return f"https://claude.ai/oauth/authorize?{urlencode(params)}" |
There was a problem hiding this comment.
PKCE code_verifier leaked via the OAuth state parameter
RFC 7636 (PKCE) requires the code_verifier to be kept secret — it should never leave the client until the token exchange, where it is sent directly over a back-channel HTTPS POST.
Here, verifier is placed in the OAuth state parameter (line 68), which travels in the front-channel authorization redirect URL:
https://claude.ai/oauth/authorize?…&state=<verifier>
This exposes the verifier in:
- The browser's address bar and history
- Server-side access logs at
claude.aiandconsole.anthropic.com - Any HTTP referrer headers sent to subsequent navigations
An attacker with access to any of those channels who also obtains the authorization code (e.g., from the shared scripts/ directory state file) can independently redeem the code without going through _exchange_code.
A separate random nonce should be used for state (for CSRF protection), and the verifier should be stored only in the local state file or in memory:
import secrets
state_nonce = secrets.token_urlsafe(16)
params = {
...
"state": state_nonce, # CSRF nonce only
"code_challenge": challenge, # derived from verifier — safe to send
"code_challenge_method": "S256",
}Then store both verifier and state_nonce in the state file and verify the nonce when the code returns.
| def _inject_system_prompt( | ||
| self, system_blocks: List[Dict[str, Any]] | ||
| ) -> List[Dict[str, Any]]: | ||
| """Prepend Claude Code identity to system prompt.""" | ||
| result = [{"type": "text", "text": CLAUDE_CODE_SYSTEM_PREFIX}] | ||
| if system_blocks: | ||
| for block in system_blocks: | ||
| result.append(block) | ||
| if len(result) >= 2: | ||
| result[1]["text"] = ( | ||
| CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"] | ||
| ) | ||
| result.pop(0) | ||
| return result |
There was a problem hiding this comment.
_inject_system_prompt mutates the caller's system_blocks[0] dict in place
result.append(block) stores a reference, not a copy. When result[1]["text"] is updated on line 365, it also updates system_blocks[0]["text"] in the caller's scope. This is currently harmless because system_blocks is discarded after this call, but it creates a subtle mutation side effect that could cause hard-to-track bugs if the call site is ever changed.
Use copy.copy(block) (or dict(block)) when building result to avoid modifying the input:
def _inject_system_prompt(self, system_blocks):
result = [{"type": "text", "text": CLAUDE_CODE_SYSTEM_PREFIX}]
if system_blocks:
for block in system_blocks:
result.append(dict(block)) # shallow copy — avoids mutating input
if len(result) >= 2:
result[1]["text"] = (
CLAUDE_CODE_SYSTEM_PREFIX + "\n\n" + result[1]["text"]
)
result.pop(0)
return result| tool_calls = msg.get("tool_calls") or [] | ||
| for tc in tool_calls: | ||
| func = tc.get("function", {}) | ||
| try: | ||
| input_data = json.loads(func.get("arguments", "{}")) | ||
| except json.JSONDecodeError: | ||
| input_data = {} | ||
| blocks.append( | ||
| { | ||
| "type": "tool_use", | ||
| "id": tc.get("id", f"toolu_{uuid.uuid4().hex[:12]}"), | ||
| "name": func.get("name", ""), | ||
| "input": input_data, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Invalid tool-call arguments silently dropped
When an OpenAI assistant message carries tool_calls with malformed arguments JSON, the json.JSONDecodeError is caught and replaced with an empty dict {}:
except json.JSONDecodeError:
input_data = {}This silently swallows a data-corruption event: the tool will be invoked with no inputs. There is no log message, no warning to the user, and no propagation of the error. Downstream tool calls may produce incorrect or misleading results. At minimum, log a warning with the offending call ID and the raw arguments string so that operators can diagnose the issue.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rotator_library/providers/anthropic_provider.py`:
- Around line 706-711: The payload sets payload["thinking"] unconditionally and
_reasoning_effort_to_budget can return invalid values (<=0 or >= max_tokens);
update _reasoning_effort_to_budget to sanitize inputs (coerce string numbers,
treat "0" or -1 as the minimum positive effort, clamp result to at least 1 and
strictly less than max_tokens), and change _build_anthropic_payload to validate
the returned budget before assigning payload["thinking"] (if budget is invalid
or >= max_tokens, either adjust it to max_tokens - 1 or omit the thinking key /
raise a clear error); reference the _reasoning_effort_to_budget function and the
payload["thinking"] assignment in _build_anthropic_payload when making these
changes.
- Around line 206-226: Duplicate base64 data-URL parsing appears in the
assistant and user message handling branches; extract it into a helper method
(e.g., _parse_image_url_block(self, url: str) -> Optional[dict]) that returns
the Anthropic image block or None for non-data URLs, then replace both inline
parsing blocks (the "image_url" branches in assistant message handling and user
message handling) with code that calls self._parse_image_url_block(url) and
appends the returned block only if it's truthy.
- Around line 933-948: The dict literal passed into litellm.ModelResponse can be
simplified by passing the keys as direct keyword arguments instead of using the
**{...} pattern; update the call to litellm.ModelResponse to pass
id=first_chunk.id, object="chat.completion", created=first_chunk.created,
model=first_chunk.model, choices=[{"index": 0, "message": final_message,
"finish_reason": finish_reason}], and usage=usage_data directly (references:
litellm.ModelResponse, first_chunk, final_message, finish_reason, usage_data).
- Around line 830-833: Replace the idiomatic error logging call in the except
block for the Anthropic stream: instead of calling lib_logger.error(...,
exc_info=True) use lib_logger.exception(...) so the traceback is logged
concisely; locate the except Exception as e handler used during the Anthropic
stream (the block that currently calls file_logger.log_error and
lib_logger.error) and change the lib_logger call to
lib_logger.exception(f"Anthropic stream error: {e}") while leaving the
file_logger.log_error and re-raise intact.
- Line 13: Update the import and type usages to modern annotations: replace the
current "from typing import Union, AsyncGenerator, List, Dict, Any, Optional"
import by importing AsyncGenerator from collections.abc (from collections.abc
import AsyncGenerator) and keeping only typing names you need (e.g., Any,
Optional, Union) from typing; then replace all occurrences of List[...] with
list[...] and Dict[...] with dict[...] in this module (look for any uses in
functions/classes such as AsyncGenerator annotations and type hints) so the code
uses built-in generics and collections.abc.AsyncGenerator.
- Around line 231-234: The code currently swallows JSONDecodeError when parsing
tool call arguments in anthropic_provider.py (the try/except around input_data =
json.loads(func.get("arguments", "{}"))), which can hide malformed payloads;
modify the except block to log a warning (using the existing logger or
self.logger) that includes the raw arguments string and the exception details,
then fall back to an empty dict as before so you preserve behavior but surface
the issue for debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 25429660-ed32-4bd4-a99e-7c3a8cc4b8dd
📒 Files selected for processing (1)
src/rotator_library/providers/anthropic_provider.py
📜 Review details
🧰 Additional context used
🪛 Ruff (0.15.4)
src/rotator_library/providers/anthropic_provider.py
[warning] 13-13: Import from collections.abc instead: AsyncGenerator
Import from collections.abc
(UP035)
[warning] 13-13: typing.List is deprecated, use list instead
(UP035)
[warning] 13-13: typing.Dict is deprecated, use dict instead
(UP035)
[error] 37-44: Consider f-string instead of string join
Replace with f-string
(FLY002)
[warning] 73-73: Missing return type annotation for private function _get_thinking_cache
(ANN202)
[warning] 92-92: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
[warning] 115-115: Do not catch blind exception: Exception
(BLE001)
[warning] 116-116: Logging statement uses f-string
(G004)
[warning] 301-301: Consider moving this statement to an else block
(TRY300)
[warning] 419-419: Missing return type annotation for private function _anthropic_event_to_openai_chunks
(ANN202)
[warning] 717-717: Dynamically typed expressions (typing.Any) are disallowed in effort
(ANN401)
[warning] 738-738: Missing type annotation for **kwargs
(ANN003)
[warning] 739-739: Use X | Y for type annotations
Convert to X | Y
(UP007)
[warning] 745-745: Missing return type annotation for private function make_request
(ANN202)
[warning] 761-761: Missing return type annotation for private function stream_handler
(ANN202)
[warning] 784-789: Abstract raise to an inner function
(TRY301)
[warning] 784-789: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 795-799: Abstract raise to an inner function
(TRY301)
[warning] 832-832: Logging .exception(...) should be used instead of .error(..., exc_info=True)
(G201)
[warning] 832-832: Logging statement uses f-string
(G004)
[warning] 835-835: Missing return type annotation for private function logging_stream_wrapper
(ANN202)
[warning] 850-850: Missing return type annotation for private function non_stream
(ANN202)
[warning] 860-860: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 934-947: Unnecessary dict kwargs
Remove unnecessary kwargs
(PIE804)
🔇 Additional comments (6)
src/rotator_library/providers/anthropic_provider.py (6)
73-81: LGTM!The lazy initialization pattern for the thinking signature cache is appropriate, with reasonable TTLs for memory (2 hours) and disk (48 hours) caching.
98-118: LGTM!The model fetching with graceful fallback to a hardcoded list is a good resilience pattern. The broad exception catch is acceptable here since any failure should trigger the fallback.
326-344: Tool choice prefixing is handled elsewhere.A previous review flagged that
_prefix_tool_namesdoesn't prefixtool_choice.name. This is addressed in_build_anthropic_payload(lines 688-693) wheretool_choiceis constructed with the prefix already applied. The overall behavior is correct.
356-369: LGTM!The system prompt injection correctly prepends the Claude Code identity prefix and merges with existing system blocks. The
len(result) >= 2check ensures safe indexing.
419-635: LGTM!The SSE event-to-chunk conversion is well-structured with clear handling for each Anthropic event type. The thinking block signature caching at
message_deltaenables multi-turn reasoning preservation.
784-789: No issues found.The
RateLimitErrorinstantiation at lines 784-789 correctly matches thelitellm.exceptions.RateLimitErrorconstructor signature. Theresponseparameter is supported as an optional argument and is being used appropriately.
| import time | ||
| import logging | ||
| import uuid | ||
| from typing import Union, AsyncGenerator, List, Dict, Any, Optional |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using modern type annotations.
The typing module imports (List, Dict, Any, Optional) are deprecated in favor of built-in generics (list, dict) and collections.abc.AsyncGenerator since Python 3.9.
♻️ Suggested modernization
-from typing import Union, AsyncGenerator, List, Dict, Any, Optional
+from collections.abc import AsyncGenerator
+from typing import Any, Optional, UnionThen replace List[...] with list[...] and Dict[...] with dict[...] throughout the file.
🧰 Tools
🪛 Ruff (0.15.4)
[warning] 13-13: Import from collections.abc instead: AsyncGenerator
Import from collections.abc
(UP035)
[warning] 13-13: typing.List is deprecated, use list instead
(UP035)
[warning] 13-13: typing.Dict is deprecated, use dict instead
(UP035)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` at line 13, Update the
import and type usages to modern annotations: replace the current "from typing
import Union, AsyncGenerator, List, Dict, Any, Optional" import by importing
AsyncGenerator from collections.abc (from collections.abc import AsyncGenerator)
and keeping only typing names you need (e.g., Any, Optional, Union) from typing;
then replace all occurrences of List[...] with list[...] and Dict[...] with
dict[...] in this module (look for any uses in functions/classes such as
AsyncGenerator annotations and type hints) so the code uses built-in generics
and collections.abc.AsyncGenerator.
| elif block.get("type") == "image_url": | ||
| url = block.get("image_url", {}).get("url", "") | ||
| if url.startswith("data:"): | ||
| parts = url.split(",", 1) | ||
| media_type = ( | ||
| parts[0] | ||
| .replace("data:", "") | ||
| .replace(";base64", "") | ||
| ) | ||
| blocks.append( | ||
| { | ||
| "type": "image", | ||
| "source": { | ||
| "type": "base64", | ||
| "media_type": media_type, | ||
| "data": parts[1] | ||
| if len(parts) > 1 | ||
| else "", | ||
| }, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract duplicated image parsing logic.
The base64 image URL parsing logic is duplicated between assistant message handling (lines 206-226) and user message handling (lines 260-276). Consider extracting a helper method to reduce duplication.
♻️ Suggested helper extraction
def _parse_image_url_block(self, url: str) -> Optional[dict]:
"""Parse a data URL into an Anthropic image block."""
if not url.startswith("data:"):
return None
parts = url.split(",", 1)
media_type = parts[0].replace("data:", "").replace(";base64", "")
return {
"type": "image",
"source": {
"type": "base64",
"media_type": media_type,
"data": parts[1] if len(parts) > 1 else "",
},
}Then use it in both locations:
elif block.get("type") == "image_url":
url = block.get("image_url", {}).get("url", "")
img_block = self._parse_image_url_block(url)
if img_block:
blocks.append(img_block)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 206 - 226,
Duplicate base64 data-URL parsing appears in the assistant and user message
handling branches; extract it into a helper method (e.g.,
_parse_image_url_block(self, url: str) -> Optional[dict]) that returns the
Anthropic image block or None for non-data URLs, then replace both inline
parsing blocks (the "image_url" branches in assistant message handling and user
message handling) with code that calls self._parse_image_url_block(url) and
appends the returned block only if it's truthy.
| try: | ||
| input_data = json.loads(func.get("arguments", "{}")) | ||
| except json.JSONDecodeError: | ||
| input_data = {} |
There was a problem hiding this comment.
Silent failure on malformed tool arguments may hide issues.
When json.loads fails on tool call arguments, the code silently defaults to an empty dict. This could mask upstream issues with malformed payloads. Consider logging a warning to aid debugging.
🛡️ Suggested fix
try:
input_data = json.loads(func.get("arguments", "{}"))
except json.JSONDecodeError:
+ lib_logger.warning(
+ f"Failed to parse tool arguments for {func.get('name', 'unknown')}, using empty dict"
+ )
input_data = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 231 - 234,
The code currently swallows JSONDecodeError when parsing tool call arguments in
anthropic_provider.py (the try/except around input_data =
json.loads(func.get("arguments", "{}"))), which can hide malformed payloads;
modify the except block to log a warning (using the existing logger or
self.logger) that includes the raw arguments string and the exception details,
then fall back to an empty dict as before so you preserve behavior but surface
the issue for debugging.
| payload["thinking"] = { | ||
| "type": "enabled", | ||
| "budget_tokens": self._reasoning_effort_to_budget( | ||
| reasoning_effort, kwargs.get("max_tokens", 16384) | ||
| ), | ||
| } |
There was a problem hiding this comment.
Edge cases still produce invalid budget_tokens values.
The previous review's concern remains partially unaddressed:
- When
max_tokens <= 1, returning1violates Anthropic'sbudget_tokens < max_tokensrequirement. - When
effortis"0"or-1, the budget can become0or negative after clamping. _build_anthropic_payload(lines 706-711) setsthinkingunconditionally without validating the budget.
🐛 Suggested fix
def _reasoning_effort_to_budget(self, effort: Any, max_tokens: int) -> int:
"""Convert reasoning_effort to Anthropic budget_tokens.
Anthropic requires budget_tokens < max_tokens, so we always clamp.
+ Returns 0 if no valid budget can be computed.
"""
+ if max_tokens <= 1:
+ return 0
effort_str = str(effort).lower().strip()
budget_map = {
"low": 4096,
"medium": 8192,
"high": 16384,
}
budget = budget_map.get(effort_str)
if budget is None:
try:
budget = int(effort_str)
except (ValueError, TypeError):
budget = 8192
- # Anthropic requires budget_tokens < max_tokens
- return min(budget, max_tokens - 1) if max_tokens > 1 else 1
+ # Anthropic requires 1 <= budget_tokens < max_tokens
+ return max(1, min(budget, max_tokens - 1))And in _build_anthropic_payload:
if reasoning_effort and str(reasoning_effort).lower() not in (
"none",
"disabled",
"off",
"false",
"disable",
):
- payload["thinking"] = {
- "type": "enabled",
- "budget_tokens": self._reasoning_effort_to_budget(
- reasoning_effort, kwargs.get("max_tokens", 16384)
- ),
- }
+ budget_tokens = self._reasoning_effort_to_budget(
+ reasoning_effort, kwargs.get("max_tokens", 16384)
+ )
+ if budget_tokens > 0:
+ payload["thinking"] = {
+ "type": "enabled",
+ "budget_tokens": budget_tokens,
+ }Also applies to: 717-735
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 706 - 711,
The payload sets payload["thinking"] unconditionally and
_reasoning_effort_to_budget can return invalid values (<=0 or >= max_tokens);
update _reasoning_effort_to_budget to sanitize inputs (coerce string numbers,
treat "0" or -1 as the minimum positive effort, clamp result to at least 1 and
strictly less than max_tokens), and change _build_anthropic_payload to validate
the returned budget before assigning payload["thinking"] (if budget is invalid
or >= max_tokens, either adjust it to max_tokens - 1 or omit the thinking key /
raise a clear error); reference the _reasoning_effort_to_budget function and the
payload["thinking"] assignment in _build_anthropic_payload when making these
changes.
| except Exception as e: | ||
| file_logger.log_error(f"Error during Anthropic stream: {e}") | ||
| lib_logger.error(f"Anthropic stream error: {e}", exc_info=True) | ||
| raise |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use lib_logger.exception() for error logging with traceback.
The logging.exception() method is preferred over logging.error(..., exc_info=True) as it's more idiomatic and concise.
♻️ Suggested fix
except Exception as e:
file_logger.log_error(f"Error during Anthropic stream: {e}")
- lib_logger.error(f"Anthropic stream error: {e}", exc_info=True)
+ lib_logger.exception("Anthropic stream error: %s", e)
raise🧰 Tools
🪛 Ruff (0.15.4)
[warning] 832-832: Logging .exception(...) should be used instead of .error(..., exc_info=True)
(G201)
[warning] 832-832: Logging statement uses f-string
(G004)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 830 - 833,
Replace the idiomatic error logging call in the except block for the Anthropic
stream: instead of calling lib_logger.error(..., exc_info=True) use
lib_logger.exception(...) so the traceback is logged concisely; locate the
except Exception as e handler used during the Anthropic stream (the block that
currently calls file_logger.log_error and lib_logger.error) and change the
lib_logger call to lib_logger.exception(f"Anthropic stream error: {e}") while
leaving the file_logger.log_error and re-raise intact.
| return litellm.ModelResponse( | ||
| **{ | ||
| "id": first_chunk.id, | ||
| "object": "chat.completion", | ||
| "created": first_chunk.created, | ||
| "model": first_chunk.model, | ||
| "choices": [ | ||
| { | ||
| "index": 0, | ||
| "message": final_message, | ||
| "finish_reason": finish_reason, | ||
| } | ||
| ], | ||
| "usage": usage_data, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying dict construction.
The litellm.ModelResponse(**{...}) pattern with a dict literal is equivalent to passing keyword arguments directly.
♻️ Simplified construction
- return litellm.ModelResponse(
- **{
- "id": first_chunk.id,
- "object": "chat.completion",
- "created": first_chunk.created,
- "model": first_chunk.model,
- "choices": [
- {
- "index": 0,
- "message": final_message,
- "finish_reason": finish_reason,
- }
- ],
- "usage": usage_data,
- }
- )
+ return litellm.ModelResponse(
+ id=first_chunk.id,
+ object="chat.completion",
+ created=first_chunk.created,
+ model=first_chunk.model,
+ choices=[
+ {
+ "index": 0,
+ "message": final_message,
+ "finish_reason": finish_reason,
+ }
+ ],
+ usage=usage_data,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return litellm.ModelResponse( | |
| **{ | |
| "id": first_chunk.id, | |
| "object": "chat.completion", | |
| "created": first_chunk.created, | |
| "model": first_chunk.model, | |
| "choices": [ | |
| { | |
| "index": 0, | |
| "message": final_message, | |
| "finish_reason": finish_reason, | |
| } | |
| ], | |
| "usage": usage_data, | |
| } | |
| ) | |
| return litellm.ModelResponse( | |
| id=first_chunk.id, | |
| object="chat.completion", | |
| created=first_chunk.created, | |
| model=first_chunk.model, | |
| choices=[ | |
| { | |
| "index": 0, | |
| "message": final_message, | |
| "finish_reason": finish_reason, | |
| } | |
| ], | |
| usage=usage_data, | |
| ) |
🧰 Tools
🪛 Ruff (0.15.4)
[warning] 934-947: Unnecessary dict kwargs
Remove unnecessary kwargs
(PIE804)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/anthropic_provider.py` around lines 933 - 948,
The dict literal passed into litellm.ModelResponse can be simplified by passing
the keys as direct keyword arguments instead of using the **{...} pattern;
update the call to litellm.ModelResponse to pass id=first_chunk.id,
object="chat.completion", created=first_chunk.created, model=first_chunk.model,
choices=[{"index": 0, "message": final_message, "finish_reason":
finish_reason}], and usage=usage_data directly (references:
litellm.ModelResponse, first_chunk, final_message, finish_reason, usage_data).
2 participants
Summary
Adds Anthropic as an OAuth provider using the PKCE authorization flow, enabling Claude Pro/Max subscription access through the proxy.
/v1/modelsendpoint with hardcoded fallbackscripts/setup_anthropic_cred.pyfor two-step OAuth credential setup (generates URL, exchanges code)Files
anthropic_auth_base.pyanthropic_provider.pyprovider_factory.pycredential_manager.pyscripts/setup_anthropic_cred.pyTest plan
🤖 Generated with Claude Code
Important
Adds Anthropic as an OAuth provider with PKCE flow, format conversion, streaming support, and credential management.
anthropic_auth_base.py.anthropic_provider.py.anthropic_provider.py./v1/modelsendpoint with fallback inanthropic_provider.py.credential_manager.py.credential_manager.py.scripts/setup_anthropic_cred.pyfor two-step OAuth credential setup.provider_factory.py.This description was created by
for 64ffb07. You can customize this summary. It will automatically update as commits are pushed.