Skip to content

Conversation

@stackloke2e
Copy link

Summary

This PR updates the ToolHive API models generated from the latest OpenAPI specification.

Changes

  • Updated Pydantic models in src/mcp_optimizer/toolhive/api_models/

Notes

  • This PR was automatically generated by the update-thv-models workflow
  • The models are generated using datamodel-codegen from ToolHive's OpenAPI endpoint

🤖 Generated with GitHub Actions

Automated update of ToolHive API models from OpenAPI specification.
@claude
Copy link

claude bot commented Nov 20, 2025

Code Review - PR #76

Summary

This PR updates auto-generated ToolHive API models from the latest OpenAPI spec. The changes are minimal and well-structured.

✅ Positive Findings

  1. Clean refactoring: RemoteAuthConfig class moved to dedicated remote.py module, improving organization
  2. New functionality: Adds OAuth 2.0 token exchange support via tokenexchange.py module
  3. Enhanced OAuth: Adds resource field to OAuth configs (RFC 8707 compliance)
  4. Better imports: Import organization in runner.py is cleaner with multi-line format

🔍 Observations

No breaking changes detected:

  • RemoteAuthConfigremote.Config is backward compatible (same fields)
  • New fields are all Optional, maintaining compatibility
  • Type changes maintain the same structure

New modules added:

  • remote.py: OAuth/remote auth configuration (39 lines)
  • tokenexchange.py: OAuth 2.0 token exchange support (40 lines)

Field additions:

  • OAuthConfig.resource (registry.py:97)
  • RunConfig.token_exchange_config (runner.py:129)
  • UpdateRegistryRequest.api_url (v1.py:22)
  • Updated UpdateRegistryRequest.allow_private_ip description to include API URL

⚠️ Recommendations

  1. Testing: Since these are auto-generated models, verify that existing code using runner.RunConfig still works correctly
  2. Documentation: Consider documenting the new token exchange feature if it's intended for external use
  3. Verification: Confirm that code importing from runner.RemoteAuthConfig (if any) is updated to use remote.Config

📊 Impact Assessment

  • Risk Level: Low (auto-generated, backward compatible)
  • Files Changed: 17 files (mostly timestamp updates)
  • Code Quality: Good (follows Pydantic best practices)
  • Convention Adherence: ✅ Uses native Python types as per CLAUDE.md

Verdict: ✅ Approve - Safe to merge after verifying no imports of the old RemoteAuthConfig class exist outside the api_models package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants