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 21, 2025

Code Review

Summary

Auto-generated Pydantic models update from ToolHive OpenAPI spec. Changes look good overall with minor observations.

Findings

✅ Strengths:

  • Clean refactoring: RemoteAuthConfig class extracted to remote.py module (better organization)
  • New tokenexchange.py module adds OAuth 2.0 token exchange support (RFC 8693)
  • Consistent timestamp updates across all files
  • New resource field added (RFC 8707 support) in OAuth configs

⚠️ Observations:

  1. Import reorganization in runner.py: The inline class RemoteAuthConfig was moved to remote.Config. This is a breaking change if external code imports runner.RemoteAuthConfig directly.

  2. Generic class naming: Both new modules use Config as the class name. While namespaced imports prevent conflicts, consider more specific names like RemoteConfig and TokenExchangeConfig for clarity when reading code.

  3. Security consideration: New fields store OAuth secrets (client_secret, client_secret_file). Ensure these are handled securely in consuming code and never logged.

  4. Type annotations: Code follows project conventions using native types (list, dict) instead of typing imports ✓

  5. New field in v1.py: api_url added to UpdateRegistryRequest - verify this does not break existing API clients.

Recommendation

Approve with notes: Changes are auto-generated and structurally sound. Monitor for any import breakage from the RemoteAuthConfig → remote.Config refactor.


🤖 Generated by Claude Code

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