Add generated Go management API client - #6637
Conversation
c7e2b2d to
f475863
Compare
jhrozek
left a comment
There was a problem hiding this comment.
Automated review focused on the hand-written surface (the SDK client wrapper, codegen tooling, and CI/Taskfile wiring) — the ~104k lines of generated ogen output were not line-reviewed. One confirmed blocker: the new sdk-go CI gate has an invalid workflow reference on both run-on-pr.yml and run-on-main.yml, so it won't actually run. A few non-blocking suggestions below.
f475863 to
2b55166
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6637 +/- ##
==========================================
- Coverage 78.97% 78.97% -0.01%
==========================================
Files 782 782
Lines 78015 78015
==========================================
- Hits 61612 61610 -2
- Misses 16398 16400 +2
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jhrozek
left a comment
There was a problem hiding this comment.
Re-checked the CHANGES_REQUESTED findings against 721ebed — all 6 code-fix items are addressed correctly (workflow refs, nil-check on doc comment, WithMaxResponseBodyBytes, drain rationale, lint rationale), and the verify.sh tmpdir cleanup suggestion was consciously declined with a documented reason. LGTM.
771eab6 to
4537c04
Compare
Summary
sdk/gomodule containing a complete Go client generated from the canonical OpenAPI contract, with all 77 documented operations, caller-provided transport support, and safe default URL, timeout, and response-size handling.task docs; add SDK verification, test, lint, and PR/main CI gates.Type of change
Test plan
task test)task test-e2e)task lint-fix)Additional verification completed:
task docstask docs-verifytask test-openapi-normalizetask sdk-testtask sdk-linttask sdk-verifytask license-checkgit diff --checktask testexercised the SDK tests successfully but did not complete cleanly in the isolated environment because unrelated race-enabled vMCP tests were reported as flaky/data-racy in an earlier run; CI should be the final root-suite authority.API Compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.Changes
sdk/go/cmd/help/openapi-normalize/cmd/help/ogen-client-wrapper/Taskfile.yml.github/workflows/docs/server/,pkg/api/v1/registry.goDoes this introduce a user-facing change?
Yes. Go developers can import a typed ToolHive management API client without bringing in ToolHive's application/runtime dependency graph. SDK release-tag automation is intentionally deferred; consumers must pin a compatible revision until a separately approved SDK release process is added.
Implementation plan
Approved implementation plan
github.com/stacklok/toolhive/sdk/go.Special notes for reviewers
AddRegistryremains generated for complete contract compatibility but accurately reflects the server's current501 Not Implementedbehavior.client.NewClientapplies a 30-second deadline when a request has none and enforces a 10 MiB response limit.NewUnsafeClientis explicitly documented for callers that supply an equivalent policy, such as streaming scenarios.Generated with Claude Code