Skip to content

Conversation

@stackloke2e
Copy link
Contributor

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 Dec 2, 2025

PR Review - Update ToolHive API Models

Summary

Auto-generated Pydantic models from ToolHive's OpenAPI spec. Changes are primarily additive with good backward compatibility.

✅ Positive Observations

  • Refactoring: RemoteAuthConfigremote.Config improves modularity (runner.py:104)
  • New Features: Added tokenexchange.Config for OAuth 2.0 token exchange (RFC 8693)
  • Backward Compatible: No breaking changes - all fields remain optional
  • Clean Structure: New modules follow existing patterns

Key Changes

  1. runner.py: Refactored inline RemoteAuthConfig class to remote.Config module, added token_exchange_config field
  2. registry.py: Added resource field for OAuth 2.0 resource indicator (RFC 8707)
  3. New modules: remote.py and tokenexchange.py for better separation of concerns
  4. v1.py: Added api_url field to UpdateRegistryRequest

🔍 Review Notes

Code Quality: ✅ Auto-generated code follows consistent patterns and uses Pydantic best practices

Breaking Changes: ✅ None - purely additive changes with optional fields

Performance: ✅ No performance impact - model definitions only

Security: ⚠️ Minor concern - New models handle OAuth secrets (client_secret, client_secret_file). Ensure:

  • Secrets are never logged
  • client_secret fields are properly redacted in error messages
  • File paths in client_secret_file are validated against path traversal

Testing: ℹ️ As these are auto-generated models, verify integration tests cover the new OAuth flows

Recommendation

LGTM - Approve and merge. Consider adding integration tests for the new token exchange functionality in a follow-up PR.

@aponcedeleonch aponcedeleonch merged commit 4f76859 into main Dec 2, 2025
6 checks passed
@aponcedeleonch aponcedeleonch deleted the update-thv-models-19842437570 branch December 2, 2025 18:52
therealnb pushed a commit that referenced this pull request Dec 3, 2025
Automated update of ToolHive API models from OpenAPI specification.

Co-authored-by: aponcedeleonch <7890853+aponcedeleonch@users.noreply.github.com>
therealnb added a commit that referenced this pull request Dec 4, 2025
…mples (#98)

* thv group investigation

Signed-off-by: nigel brown <nigel@stacklok.com>

* Example tweaks

Signed-off-by: nigel brown <nigel@stacklok.com>

* Skip registry if in k8s mode

Signed-off-by: nigel brown <nigel@stacklok.com>

* URL normalization for streamable-http transport
For streamable-http:
  Strips URL fragments (#github → removed)
  Converts /sse paths to /mcp
  Ensures path ends with /mcp if missing
For SSE:
  preserves original URL (fragments needed for container identification)

Signed-off-by: nigel brown <nigel@stacklok.com>

* Update ToolHive API models (#92)

Automated update of ToolHive API models from OpenAPI specification.

Co-authored-by: aponcedeleonch <7890853+aponcedeleonch@users.noreply.github.com>

* chore(deps): update actions/checkout action to v6 (#77)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update peter-evans/create-pull-request action to v7.0.9 (#81)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update docker/metadata-action action to v5.10.0 (#87)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update renovate.json to bump pyproject.toml dependencies (#93)

* Fix formatting issues

* Fix URL normalization to avoid double-adding /mcp

- Updated normalization logic to check if /mcp already exists in path before adding it
- Updated test expectations to account for URL normalization behavior
- Fixes failing tests where URLs like /mcp/test-server were getting /mcp appended

---------

Signed-off-by: nigel brown <nigel@stacklok.com>
Co-authored-by: stackloke2e <160783949+stackloke2e@users.noreply.github.com>
Co-authored-by: aponcedeleonch <7890853+aponcedeleonch@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Alejandro Ponce de Leon <aponcedeleonch@stacklok.com>
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