fix(sdk): sync generator templates with hand-edited SDK output#884
Merged
mergify[bot] merged 5 commits intomainfrom Mar 11, 2026
Merged
fix(sdk): sync generator templates with hand-edited SDK output#884mergify[bot] merged 5 commits intomainfrom
mergify[bot] merged 5 commits intomainfrom
Conversation
The generated Go SDK had hand-edits that would be lost on regeneration: - WithInsecureSkipVerify client option (TLS skip for self-signed certs) - Compiled bearerTokenPattern regex (was per-call MustCompile) - Sessions().Delete() method (missing DELETE in OpenAPI spec) - Extra blank lines in type builders (template whitespace) Fix by updating the source templates and OpenAPI spec so regeneration produces identical output (minus timestamp/hash). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (38)
WalkthroughAdds a DELETE operation to the sessions OpenAPI spec, introduces an HTTP client option to skip TLS verification and improves bearer-token redaction in the Go SDK generator template, tightens whitespace in the Go types generator template, and adds a GitHub Actions workflow to detect SDK drift by regenerating and diffing SDKs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Regenerates the SDK from the OpenAPI spec on PRs that touch the generator, templates, SDK output, or OpenAPI specs. Fails if the regenerated output differs from what's committed (ignoring timestamps). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ambient-api-server/openapi/openapi.sessions.yaml`:
- Around line 193-198: The OpenAPI docs advertise a 403 but the DeleteSession
handler never returns it; update the DeleteSession handler to explicitly detect
a forbidden RBAC/Kubernetes error (e.g. apierrors.IsForbidden(err) or matching
the Forbidden status) and return an HTTP 403 using the same Error response
shape, ensuring the response matches the openapi.sessions.yaml Error schema;
alternatively, if you prefer docs change, remove the 403 entry from
openapi.sessions.yaml so the spec matches current DeleteSession behavior.
In `@components/ambient-sdk/generator/templates/go/http_client.go.tmpl`:
- Around line 44-51: The WithInsecureSkipVerify ClientOption currently replaces
c.httpClient.Transport with a new *http.Transport, losing defaults like
ProxyFromEnvironment and HTTP/2 support; instead, retrieve the existing
transport (from c.httpClient.Transport or http.DefaultTransport), clone it into
a new *http.Transport, ensure TLSClientConfig is non-nil, set
TLSClientConfig.InsecureSkipVerify = true on the clone, and assign the cloned
transport back to c.httpClient.Transport so only the TLS setting is changed
while other defaults are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: faa9fb3e-cccd-43fb-82af-4fdf99e1ebeb
📒 Files selected for processing (3)
components/ambient-api-server/openapi/openapi.sessions.yamlcomponents/ambient-sdk/generator/templates/go/http_client.go.tmplcomponents/ambient-sdk/generator/templates/go/types.go.tmpl
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sdk-drift-check.yml:
- Around line 67-84: The workflow currently runs the same git diff twice when
DRIFT is detected; change the logic to run git diff once, capture its output
into a variable or temp file (e.g., store into DRIFT or DIFF_OUTPUT), then test
if the variable is non-empty and print that captured diff in the echo/Display
section before exit 1; update the conditional that checks -n "$DRIFT" to use the
captured content and remove the second git diff invocation so only a single git
diff call (the one that populates the variable/file) is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b1b2e597-d893-4036-b294-7d0ad8569d78
📒 Files selected for processing (1)
.github/workflows/sdk-drift-check.yml
Regenerate all SDK outputs (Go, Python, TypeScript) so committed files match what the generator produces with the updated OpenAPI spec (session DELETE endpoint) and templates. Fix drift check ignore patterns to match both // and # comment styles for timestamp/hash lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The generator templates produce trailing blank lines that pre-commit's end-of-file-fixer strips on commit, causing perpetual drift. Fix by trimming trailing newlines in the generator itself so output always ends with exactly one newline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t check - Clone the existing http.Transport instead of replacing it, preserving ProxyFromEnvironment, HTTP/2, and connection pool defaults - Remove duplicate git diff call in SDK drift check workflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge Queue Status
This pull request spent 6 seconds in the queue, with no time running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make generate-sdkproduces output identical to the current hand-edited SDK codeChanges
generator/templates/go/http_client.go.tmplcrypto/tlsimportinsecureSkipVerifyfield toClientstructWithInsecureSkipVerify()client option (needed for OpenShift/self-signed certs)bearerTokenPatternregex at package level (wasMustCompileper call)ambient-api-server/openapi/openapi.sessions.yamlDELETE /api/ambient/v1/sessions/{id}endpoint so the generator emitsSessions().Delete()generator/templates/go/types.go.tmpl{{range}}blocksVerification
After these changes, running the generator produces output that differs from the committed SDK only in the timestamp and spec hash:
Test plan
go run . -spec ...succeeds, reportsSession delete=trueclient.goretainsWithInsecureSkipVerify,insecureSkipVerify, compiled regexsession_api.goincludesDelete()methodgofmtDeletemethod position🤖 Generated with Claude Code