fix: preserve AuthError type in oauth_http_client cache

[G7CNF]

Summary

• change oauth_http_client() static cache type from Result<Client, String> to Result<Client, AuthError>
• map reqwest client build failures directly to AuthError::Http
• return cached errors via clone without string round-tripping

Why

Issue #1141 reported that we were storing stringified errors in the static cache and only re-wrapping at read time. This loses typed error context in the cache path and adds unnecessary conversion.

Fixes #1141

Validation

• cargo test -p ironclaw tools::mcp::auth::tests::test_auth_error_display_strings -- --nocapture
• cargo clippy -p ironclaw --all-targets -- -D warnings

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the oauth_http_client function to maintain proper error typing within its static cache. By directly storing and retrieving AuthError instances, it eliminates intermediate string conversions and ensures that the full context of authentication-related errors is preserved throughout the application, addressing a reported issue where error context was lost.

Highlights

• Error Type Preservation: The static cache for oauth_http_client() now stores Result<Client, AuthError> instead of Result<Client, String>, preserving the original AuthError type.
• Direct Error Mapping: Failures during reqwest client building are now directly mapped to AuthError::Http, ensuring consistent error handling.
• Simplified Error Cloning: Cached errors are now returned via Clone::clone without unnecessary string conversions, improving efficiency and type safety.

Changelog

• src/tools/mcp/auth.rs
  • Updated the CLIENT static OnceLock type to store Result<reqwest::Client, AuthError> instead of Result<reqwest::Client, String>.
  • Modified the error mapping for reqwest::Client::builder().build() to directly convert build errors into AuthError::Http.
  • Changed the error retrieval from the OnceLock to use Clone::clone on the AuthError variant, removing the need for string round-tripping.
  • Derived the Clone trait for the AuthError enum to enable cloning of cached errors.

Activity

• No specific review comments or activities have been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the oauth_http_client function to cache the AuthError type directly, instead of just a string representation of the error. This is a good improvement as it preserves the typed error information in the cache path and avoids unnecessary string conversions. The changes are correct and well-implemented, including deriving Clone on the AuthError enum to support this caching strategy. The suggestion to improve error semantics further by introducing a more specific AuthError variant for the client build failure, instead of reusing AuthError::Http, aligns with best practices for clear error handling and has been retained.

[henrypark133]

Review: typed cache error is cleaner and the scope stays tight

This is a small improvement, but it moves the cache path in the right direction: the static client cache now keeps AuthError instead of immediately erasing it to String and re-wrapping later.

Positives:

• The change is narrowly scoped to the cache path.
• Deriving Clone on AuthError is reasonable for the current enum shape.
• Meaningful CI is green.

No blocking issues from my side.