feat: adds ambient-sdk w/ python,go,typescript sdks by markturansky · Pull Request #677 · ambient-code/platform

markturansky · 2026-02-24T18:06:30Z

Based on new ambient-api-server:

Implements SDK generator based on openapi.yaml
Implements Go/Python/TypeScript SDKs

github-actions · 2026-02-24T18:16:10Z

Claude Code Review

Summary

PR #677 adds a multi-language SDK (components/ambient-sdk/) with a Go-based code generator that reads openapi.yaml and produces Go, Python, and TypeScript client libraries. The architecture is solid — generator + templates producing typed, zero-K8s-dependency SDKs. However, there are two critical blockers: the core HTTP client implementations are missing from both the Go and Python SDKs, leaving them non-compilable/non-importable as merged.

Issues by Severity

🚫 Blocker Issues

1. Go SDK is missing its core HTTP client (go-sdk/client/client.go)

All five generated API files (project_api.go, session_api.go, user_api.go, etc.) call a.client.do() and a.client.doWithQuery(), but no Client struct exists in the go-sdk/client/ package. The SDK's CLAUDE.md explicitly lists client/client.go as a key file ("HTTP client with structured logging and token sanitization"), but it is absent from both the PR diff and the repository. The Go SDK will not compile.

$ go build ./...
# undefined: Client

2. Python SDK is missing its core HTTP client (python-sdk/ambient_platform/client.py)

All generated API files call self._client._request(...) (e.g., _session_api.py:24), and tests/test_client.py imports from ambient_platform.client import AmbientClient — but there is no client.py in ambient_platform/. The Python SDK cannot be imported and all tests will fail with ModuleNotFoundError.

3. Python SDK is missing __init__.py

ambient_platform/ has no __init__.py, so it is not a valid Python package. pip install -e . will not expose the package for import.

🔴 Critical Issues

4. TypeScript client.test.ts references resources that don't exist in generated client.ts

tests/client.test.ts:13 asserts client.agents, client.workflows, client.tasks, client.skills, client.workflowSkills, client.workflowTasks, client.permissions, client.repositoryRefs, client.projectKeys exist. The generated client.ts only exposes projects, projectSettings, sessions, users. This test will fail:

TypeError: Cannot read properties of undefined (reading 'create')

5. URL injection risk in all Go SDK ID path construction

All Get(), Delete(), and action methods build URLs via string concatenation without path-escaping the id parameter:

// go-sdk/client/session_api.go:37
a.client.do(ctx, http.MethodGet, "/sessions/"+id, ...)

If id contains /, ?, or #, it corrupts the request path. Use url.PathEscape(id) or net/url.JoinPath().

6. Generator tests are absent

generator.md documents an extensive golden-file test suite (generator/generator_test.go) as the primary correctness guarantee, but this file does not exist. The generator itself has zero test coverage — any template change can silently produce incorrect SDK code.

🟡 Major Issues

7. strings.Title deprecated in generator

generator/main.go uses strings.Title as a template function, which is deprecated since Go 1.18 and will fire a SA1019 lint warning under golangci-lint run (required by CLAUDE.md pre-commit checklist). Replace with golang.org/x/text/cases.

8. itoa() reimplementation instead of stdlib

go-sdk/types/base.go (and the base.go.tmpl template) manually implements integer-to-string conversion. Use strconv.Itoa() or fmt.Sprintf("%d", i) to avoid a maintenance footprint.

9. No SDK CI pipeline

None of the existing GitHub Actions workflows build, lint, or test the SDK components. The generator.md describes a CI drift-detection job but there is no corresponding workflow file. Without CI enforcement, generated files will drift from the spec and the SDKs will break silently.

10. go-sdk/README.md references stale API surface

The README documents NewClient(baseURL, token, project string) *Client, CreateSession(ctx, req *CreateSessionRequest), WaitForCompletion(...) — none of which exist in the generated code. The actual API uses NewSessionBuilder(), c.Sessions().Create(ctx, session), etc. First-time users will be immediately blocked.

🔵 Minor Issues

11. Python list_all accepts unused **kwargs

# python-sdk/ambient_platform/_session_api.py:47
def list_all(self, size: int = 100, **kwargs: Any) -> Iterator[Session]:

**kwargs is accepted but silently dropped. This misleads callers who might pass page=5 expecting it to set the start page. Either use the kwargs or remove the parameter.

12. Python APIError combines frozen=True dataclass with Exception

@dataclass(frozen=True) class APIError(Exception) is unusual and can cause subtle issues: frozen dataclasses override __setattr__, which conflicts with Exception's internal mutation during pickling and with_traceback(). Consider a regular Exception subclass with typed attributes instead.

13. Python ListOptions.to_params() skips falsy values including 0

return {k: v for k, v in self._params.items() if v}

This silently drops page=0 or size=0 if either is ever set to those values. Use if v is not None for robustness.

14. TypeScript ambientFetch response body cast is unsafe

errorData = await resp.json() as APIError;  // ts-sdk/src/base.ts:97

and

return resp.json() as Promise<T>;  // base.ts:118

These are unchecked casts. While unavoidable at the HTTP boundary, the error path silently accepts malformed server responses. Consider adding a status_code field presence check on error responses.

15. kustomization.yaml comment moved but not fixed

The production manifest comment move (# PostgreSQL: use RHEL image...) is cosmetically fine, but also repositions fields (version: v1) — ensure this is not a Kustomize version where field order matters.

Positive Highlights

Generator design is clean: Go text/template with per-resource data context maps directly to the K8s code generator convention. SHA256 spec hash in file headers enables offline drift detection.
TypeScript SDK is the most complete: base.ts, client.ts, full type exports in index.ts, and three test files with meaningful coverage. The AmbientAPIError class with structured fields is well-designed.
Python input validation is thorough: test_client.py covers empty token, short token, placeholder token, invalid URL scheme, and placeholder domains — all correct security controls for an external SDK.
Pagination iterator design: The Go generic Iterator[T] and Python list_all() generator are idiomatic and reusable.
Builder pattern with required-field validation: All three SDKs enforce required fields at build() time rather than at HTTP request time.
Zero K8s dependencies: Go SDK go.mod has no dependencies; Python uses only httpx. This keeps the external SDK footprint minimal.
Context propagation: All Go SDK methods accept context.Context as first argument, which is correct.

Recommendations

Before merge (blockers): Add go-sdk/client/client.go and python-sdk/ambient_platform/client.py (the actual HTTP transport layer), and add python-sdk/ambient_platform/__init__.py.
Before merge (critical): Fix client.test.ts to match the actual AmbientClient API surface, or expand client.ts to include the resources the test expects.
Before merge (critical): Add url.PathEscape(id) for all ID-based URL construction in the Go SDK templates and generated files.
Follow-up: Add generator/generator_test.go golden-file tests (described in generator.md but absent).
Follow-up: Add a GitHub Actions workflow that builds Go/Python/TS SDKs and runs their tests on every PR touching components/ambient-sdk/.
Follow-up: Fix strings.Title deprecation in generator to pass golangci-lint run.

🤖 Generated with Claude Code

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-24T19:46:57Z

Claude Code Review

Summary

PR #677 introduces a multi-language SDK generator and generated Go, Python, and TypeScript client libraries for the Ambient Platform public API. The overall architecture is sound — OpenAPI-spec-driven generation, consistent builder patterns, good security defaults for token handling, and proper pagination iterators across all three languages. However, there are critical bugs that prevent the Go SDK from compiling, a functional routing bug in the TypeScript SDK, and a public API typo in the Python SDK.

Issues by Severity

🚫 Blocker Issues

1. Go SDK does not compile — types.ErrorResponse undefined

components/ambient-sdk/go-sdk/client/client.go:147 references types.ErrorResponse, but this type is not defined anywhere in the types/ package. The types/ directory contains only generated files (base.go, session.go, project.go, project_settings.go, user.go, list_options.go) and none define ErrorResponse.

// client.go:147 — COMPILE ERROR: types.ErrorResponse undefined
var apiErr types.ErrorResponse

The README.md documents this type as:

type ErrorResponse struct { Error string; Details string }

...but this struct was never generated into the types package. The generator template for client.go is out of sync with the base.go template.

2. Go SDK does not compile — types.APIError struct field mismatch

client.go:151,157 constructs types.APIError with Message and Details fields. But types/base.go defines APIError with Code, Reason, OperationID, StatusCode — no Message or Details fields.

// client.go:151-158 — COMPILE ERROR: unknown fields Message, Details
return &types.APIError{
    StatusCode: resp.StatusCode,
    Message:    apiErr.Error,   // field does not exist
    Details:    apiErr.Details, // field does not exist
}

Both blockers are in the same generated file and stem from the client.go generator template being written against a different APIError shape than what base.go generates. Fix: add ErrorResponse to the types template and align field names, or update the client.go template to use the generated APIError{Code, Reason} fields directly.

🔴 Critical Issues

3. TypeScript SDK hits wrong API paths — missing /api/ambient-api-server/v1 prefix

The Go and Python SDKs prepend the base path before every request:

// client.go
url := c.baseURL + "/api/ambient-api-server/v1" + path

# client.py
url = urljoin(self._base_url + "/", f"api/ambient-api-server/v1{path}")

The TypeScript ambientFetch in base.ts does not:

// base.ts
const url = `${config.baseUrl}${path}`;
// session_api.ts calls: ambientFetch(config, 'POST', '/sessions', ...)
// Result: http://localhost:8080/sessions  ← WRONG
// Expected: http://localhost:8080/api/ambient-api-server/v1/sessions

TypeScript users with AMBIENT_API_URL=http://localhost:8080 (the documented default) will receive 404s on every call. The TypeScript SDK is functionally broken for any user following the README defaults.

Fix: add the prefix in ambientFetch or in AmbientClient.fromEnv().

4. Developer's local filesystem path in every generated file header

All generated files include:

// Source: /home/mturansk/projects/src/github.com/ambient/platform/components/...

This leaks the original developer's home directory path to every SDK consumer. Remove the Source: line from all generator templates; the Spec SHA256 line is sufficient for traceability.

🟡 Major Issues

5. Python project_settingss() method — public API typo

components/ambient-sdk/python-sdk/ambient_platform/client.py:161:

def project_settingss(self) -> ProjectSettingsAPI:  # double-s

This is a generator bug: the template naively appends s to ProjectSettings. Once released and documented, renaming this is a breaking change. Fix the generator's pluralization logic for resources that already end in s (e.g., return project_settings_api as the accessor name, or use a custom accessor-name override in the resource definition).

6. sanitizeLogURL regex is misapplied and effectively dead code

components/ambient-sdk/go-sdk/client/client.go:

func sanitizeLogURL(rawURL string) string {
    tokenPattern := regexp.MustCompile(`([Bb]earer\s+)[a-zA-Z0-9\-_~.+/=]+`)
    return tokenPattern.ReplaceAllString(rawURL, "${1}[REDACTED]")
}

This function applies a Bearer-token redaction regex to URL strings. Tokens appear in Authorization headers, not in URL paths — this regex will never match anything in practice. The function is misleadingly named and provides no actual sanitization. The real security win (Authorization header not logged) is already correct by omission; just remove or rename this function, or delete it.

7. CLAUDE.md contains development scaffolding — coordinator server at localhost:4345

components/ambient-sdk/CLAUDE.md instructs AI agents to POST updates to a coordinator server:

Post your update: curl -s -X POST http://localhost:4345/agent/sdk ...

This is active-development scaffolding that should not be committed to the repository. External contributors and CI pipelines should not see (or attempt to reach) a local development server. Remove or replace with standard contribution guidelines before merge.

8. README.md shows incorrect Go module path

components/ambient-sdk/README.md Quick Start shows:

go get github.com/ambient/platform-sdk

The actual module path (from go.mod) is:

github.com/ambient-code/platform/components/ambient-sdk/go-sdk

This will mislead users attempting to install the SDK. Also, the README still lists Python and TypeScript SDKs as "Phase 3" / "future" checkboxes, but both are already implemented in this PR.

🔵 Minor Issues

9. Python re module imported but unused in client.py

components/ambient-sdk/python-sdk/ambient_platform/client.py:9:

import re  # never referenced in client.py

Dead import from the generator template. Remove it.

10. doWithQuery omits OrderBy and Fields from query parameters

client.go's doWithQuery builds query params from ListOptions but only includes page, size, and search — omitting OrderBy and Fields that are part of the ListOptions struct. These will silently be ignored when passed by callers.

11. Hand-rolled itoa in types/base.go — unnecessary

components/ambient-sdk/go-sdk/types/base.go implements a custom integer-to-string function instead of using strconv.Itoa. strconv is Go standard library and incurs no external dependency cost. The hand-rolled version adds maintenance surface for no benefit.

12. TypeScript size cap is a magic number

base.ts:

params.set('size', String(Math.min(opts.size, 65500)));

65500 is undocumented. Promote it to a named constant with a comment explaining the API server's limit.

13. generator.md appears to be a development artifact

components/ambient-sdk/generator.md is present at the SDK root. If this is a design document it should be in docs/; if it's a scratch file it should be deleted.

Positive Highlights

Token security is well-designed: Go SDK's SecureToken type implements slog.LogValuer for automatic log redaction. Python's _request method sets the Authorization header directly without ever logging the token value. TypeScript similarly avoids logging auth headers. The security-first approach to token handling is correct and consistent.
Builder pattern across all 3 SDKs: The SessionBuilder/SessionPatch pattern provides a nice ergonomic API that prevents partial-object mistakes and validates required fields before sending.
Automatic pagination via iterators: ListAll/list_all/listAll in all three SDKs handles page-walking transparently. The TypeScript AsyncGenerator implementation is especially clean.
Input validation at construction time: All three SDKs reject empty tokens, placeholder URLs (example.com), and unsupported schemes at NewClient time rather than at first request, giving fast failure.
OpenAPI-driven generation with SHA256 traceability: Using a spec hash in the file header is a good practice for detecting drift between spec and generated code.
Context/cancellation support: Go uses context.Context throughout; TypeScript exposes AbortSignal via RequestOptions. Python's httpx supports cancellation via timeouts. All correct.
TypeScript uses proper types throughout: No any in the generated TypeScript files — uses typed request/response objects and well-structured generics. Consistent with CLAUDE.md's zero-any requirement.

Recommendations

Prioritized action items before merge:

[Blocker] Fix generator client.go template to define ErrorResponse in the types package (or eliminate the intermediate struct) and align APIError field names with base.go.
[Blocker] Run go build ./... in CI for the Go SDK to catch compile errors before merge.
[Critical] Fix TypeScript ambientFetch to prepend /api/ambient-api-server/v1 to paths, matching Go and Python behavior.
[Critical] Strip the // Source: /home/mturansk/... line from all generator templates.
[Major] Fix generator pluralization for ProjectSettings — the Python project_settingss() accessor must be corrected before this becomes a released API.
[Major] Remove coordinator server instructions from CLAUDE.md.
[Major] Correct the README Quick Start Go module path and update the implementation status checklist.
[Minor] Add go build ./... and a Python import smoke test (python -c "from ambient_platform import *") to CI for the SDK.

🤖 Generated with Claude Code

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-24T21:19:30Z

Claude Code Review

Summary

This PR introduces a new components/ambient-sdk/ component containing a code generator and three generated SDK implementations (Go, Python, TypeScript) for the Ambient Platform public REST API. It also includes a minor fix to components/manifests/overlays/production/kustomization.yaml. The concept is strong — a generated, API-first SDK is the right approach for public consumption. However, there are several issues ranging from a critical test/implementation mismatch to minor quality issues that should be addressed before merge.

Issues by Severity

🚫 Blocker Issues

1. Python test suite references non-existent API accessors — tests will fail

tests/test_client.py (TestClientResourceAccessors) asserts the existence of agents, tasks, skills, workflows, workflow_skills, workflow_tasks, permissions, repository_refs, project_keys — none of which exist in the generated client.py. The generated client only exposes projects(), project_settingss(), sessions(), and users(). These tests will fail on import or assertion.

The tests appear to have been written for a broader OpenAPI spec that includes more resources than are currently in the generated output. Either the tests need to be pruned to match the current generated code, or the spec needs to be expanded to generate the missing resources.

2. Python SDK public API typo: project_settingss() (double-s)

components/ambient-sdk/python-sdk/ambient_platform/client.py:161

def project_settingss(self) -> ProjectSettingsAPI:   # ← typo: extra 's'

This is a public method name bug in a generated SDK. The generator template (generator/templates/python/http_client.py.tmpl) is producing a malformed method name. Callers will get AttributeError on client.project_settings. Fix the template, not just the output file.

3. Token validation missing in generated Python client, contradicting tests

_validate_config() in client.py validates the base URL and project but never validates the token. Yet the test suite includes:

test_empty_token_raises (expects ValueError: token cannot be empty)
test_short_token_raises (expects ValueError: too short)
test_placeholder_token_raises (expects ValueError: placeholder)

These tests will fail because no token validation exists in the generated client. The CLAUDE.md for this component also states: "Tokens are validated on client construction". The token validation needs to be added to the generator template.

🔴 Critical Issues

4. Developer's local filesystem paths leaked in all generated file headers

Every generated file (Go, Python, TypeScript) contains:

// Source: /home/mturansk/projects/src/github.com/ambient/platform/...

This leaks the developer's username (mturansk) and local machine path structure into committed, public-facing SDK code. The generator template should use a canonical relative path (e.g., components/ambient-api-server/openapi/openapi.yaml) or just omit the absolute source path entirely.

5. Developer cluster URL and username in committed README

components/ambient-sdk/go-sdk/examples/README.md contains:

export AMBIENT_API_URL="https://public-api-route-mturansk.apps.rosa.xezue-pjejw-oy9.ag90.p3.openshiftapps.com"
export AMBIENT_PROJECT="mturansk"

This exposes a specific developer's production OpenShift cluster hostname. Replace with a placeholder like https://your-ambient-platform.example.com and your-project-name.

6. Go SDK token stored as plain string, not SecureToken

The Client struct (client/client.go) stores token string, but both the component's CLAUDE.md and docs/architecture.md claim the Go SDK uses SecureToken with slog.LogValuer for automatic log redaction. The actual implementation contradicts the documentation. While sanitizeLogURL provides some defense-in-depth, the token could still appear in raw form in custom slog handlers. Either implement the documented SecureToken type or update the docs to accurately describe what's implemented.

🟡 Major Issues

7. Generator: computeSpecHash hardcodes specific resource filenames

generator/main.go:346-363 hardcodes:

files := []string{
    specPath,
    filepath.Join(specDir, "openapi.sessions.yaml"),
    filepath.Join(specDir, "openapi.projects.yaml"),
    // ...
}

If a new resource is added (e.g., openapi.agents.yaml), the hash won't include it, making the drift-detection mechanism unreliable. Use filepath.Glob(filepath.Join(specDir, "openapi.*.yaml")) instead.

8. Go SDK: url variable shadows the net/url package import in do()

client/client.go:

func (c *Client) do(ctx context.Context, method, path string, ...) error {
    url := c.baseURL + "/api/ambient-api-server/v1" + path  // shadows "url" package

The net/url package is imported and used in doWithQuery. While Go resolves this correctly due to scoping, this is a code quality issue that may confuse readers. Rename the local variable to reqURL or fullURL.

9. README.md module paths do not match actual module

components/ambient-sdk/README.md shows:

go get github.com/ambient/platform-sdk  // ← wrong
import "github.com/ambient/platform-sdk/client"  // ← wrong

The actual Go module is github.com/ambient-code/platform/components/ambient-sdk/go-sdk. The README appears to be from an earlier draft and was not updated. Also, types.internal_types referenced in the same README does not exist.

10. strings.Title is deprecated in Go 1.18+

generator/main.go:302 uses strings.Title which has been deprecated since Go 1.18 in favor of golang.org/x/text/cases. This generates a deprecation warning. Since the generator outputs code used as a public SDK, tool warnings should be addressed.

11. No CI coverage for the new SDK component

No CI workflow is added or modified to run go test, pytest, or the TypeScript test suite for components/ambient-sdk/. Without CI, regressions in the generator or SDKs won't be caught automatically. A sdk-tests.yml workflow (or additions to existing workflows) should gate merges.

12. Default port inconsistency between SDKs

Go SDK NewClientFromEnv: defaults to http://localhost:8000
Python SDK from_env(): defaults to http://localhost:8080
TypeScript SDK fromEnv(): defaults to http://localhost:8080

CLAUDE.md for the SDK component states the default is http://localhost:8080. The Go SDK is inconsistent.

🔵 Minor Issues

13. Custom itoa() in go-sdk/types/base.go reinvents strconv.Itoa

The 20-line hand-rolled itoa() function should be replaced with the standard strconv.Itoa(i). The stated goal of "standard library only" doesn't apply here since strconv is part of the standard library.

14. TypeScript SDK lacks URL validation present in Go and Python SDKs

Go and Python SDKs reject example.com and non-HTTP schemes at construction time. The TypeScript AmbientClient constructor performs no URL validation. This is a minor inconsistency but means placeholder URLs will silently produce failed requests in TypeScript.

15. Ignored error in Go example json.Marshal

go-sdk/examples/main.go:74:

reposJSON, _ := json.Marshal(...)  // error silently discarded

Example code is often copy-pasted. Unhandled errors in examples encourage bad practices.

16. pyproject.toml classifier claims Production/Stable for unreleased SDK

"Development Status :: 5 - Production/Stable",

The SDK is new and the smoke test currently returns 404 (per CLAUDE.md). 4 - Beta or 3 - Alpha is more accurate.

17. Generator template path resolution is fragile

getTemplateDir() resolves templates relative to the executable binary path. This will break when the generator is run from different working directories or in CI containers. Embedding the templates with //go:embed would be more robust.

18. CLAUDE.md coordinator server references left in committed file

components/ambient-sdk/CLAUDE.md contains:

ALWAYS report changes, suggestions, and next steps in ../working.md
...
The coordinator server at http://localhost:4345 manages ../working.md

These appear to be AI agent coordination instructions from an active development session and were likely not intended to be committed as permanent documentation. They should be removed before the SDK is public-facing.

Positive Highlights

Architecture is well-reasoned. API-first, OpenAPI-driven code generation is the right pattern for multi-language SDK consistency. The generator is functional and produces clean, idiomatic code.
Security-conscious design. Token redaction (sanitizeLogURL), URL validation rejecting placeholder domains, and structured error responses without leaking internals are all good patterns.
Iterator pattern is solid. The Go Iterator[T] and TypeScript AsyncGenerator approaches are idiomatic and avoid forcing callers to manage pagination manually.
Context-aware HTTP requests. All Go HTTP calls use http.NewRequestWithContext, correctly propagating cancellation and deadlines.
Python dataclass approach is appropriate. Using frozen dataclasses instead of Pydantic keeps the dependency footprint minimal, which is consistent with the "lightweight single dependency" design goal.
Test coverage for constructors is good. Both Python and TypeScript have solid unit tests for construction validation, fromEnv() factories, and error handling.
Kustomization cleanup is correct. The YAML reordering fixes a comment misplacement and field ordering issue in a non-breaking way.

Recommendations

Priority order:

Fix project_settingss() typo in generator template (blocker — breaks public API)
Align Python tests with generated code OR add token validation to generator template (blocker — CI will fail)
Strip developer paths from generated file headers via the generator template
Remove/replace the developer cluster URL from examples/README.md
Implement SecureToken (or update docs to reflect actual behavior)
Fix computeSpecHash to use glob instead of hardcoded filenames
Add CI workflow for SDK tests
Fix default port inconsistency (Go should match Python/TS at 8080)
Remove coordinator server instructions from committed CLAUDE.md
Address remaining minor issues before public SDK release

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-25T04:55:03Z

Claude Code Review

Summary

PR #677 introduces a new ambient-sdk component with Go, Python, and TypeScript client libraries generated from an OpenAPI spec, plus the generator itself. The design philosophy is solid (HTTP-first, zero-K8s dependencies, token security focus), and the generator architecture is well-structured. However, there are several blockers that need resolution before merging, primarily around the Python SDK having duplicate files, test failures, and a developer machine path embedded in committed files.

Issues by Severity

🚫 Blocker Issues

1. Developer machine path embedded in generated Python files

python-sdk/ambient_platform/client.py:3 and _base.py:3 contain:

# Source: /home/mturansk/projects/src/github.com/ambient/platform/...

The Go SDK correctly uses a relative path (../../ambient-api-server/openapi/openapi.yaml). The Python generator template needs to use a relative path. This exposes a developer's machine layout and will be wrong for every contributor.

2. Python SDK is duplicated — generator bug

Files exist at BOTH python-sdk/ root AND python-sdk/ambient_platform/:

client.py, session.py, project.py, user.py, project_settings.py
_base.py, _iterator.py, _session_api.py, _project_api.py
__init__.py

The generator writes to python-sdk/ambient_platform/ (correct), but stale/duplicate files at the package root will shadow the actual package and cause import confusion.

3. Test suite in test_client.py will fail — references nonexistent accessors

# test_client.py:149-178 — all of these fail with AttributeError
def test_agents_accessor(self):
    assert self.client.agents is not None      # AmbientClient has no .agents

def test_tasks_accessor(self):
    assert self.client.tasks is not None       # AmbientClient has no .tasks

def test_skills_accessor(self):
    assert self.client.skills is not None      # AmbientClient has no .skills

The generated AmbientClient only exposes projects(), project_settingss(), sessions(), and users(). The tests were written against a different, more complete client.

4. Token validation tests will fail — _validate_config doesn't validate tokens

test_client.py:9-23 expects ValueError("token cannot be empty") for an empty token, but _validate_config() only validates base_url and project. Token is never validated, so tests for short tokens and placeholder tokens also fail.

5. _base_path attribute test will raise AttributeError

test_client.py:81:

assert client._base_path == "/api/ambient-api-server/v1"

No _base_path attribute exists on AmbientClient. The base path is inlined in _request().

🔴 Critical Issues

6. project_settingss typo in Python client (double 's')

python-sdk/ambient_platform/client.py:161:

def project_settingss(self) -> ProjectSettingsAPI:   # typo: double 's'

Should be project_settings. All tests checking self.client.project_settings will fail with AttributeError since Python looks up the attribute name, not the double-s method.

7. Go client stores token as plain string — CLAUDE.md promises SecureToken

go-sdk/client/client.go:28:

type Client struct {
    token   string  // plain string, no SecureToken wrapper

The component's own CLAUDE.md states: "SecureToken type wraps tokens and implements slog.LogValuer for automatic redaction" — but SecureToken is never implemented or used. The token could accidentally leak via fmt.Sprintf("%v", client). At minimum, add a String() method to Client that redacts this field.

8. Generator uses deprecated strings.Title

generator/main.go:301:

"title": strings.Title,

Deprecated since Go 1.18. golangci-lint will flag this. Use golang.org/x/text/cases.

🟡 Major Issues

9. README Go module path is wrong

README.md:11: go get github.com/ambient/platform-sdk — but actual go.mod module is github.com/ambient-code/platform/components/ambient-sdk/go-sdk. Will mislead users.

10. README directory structure doesn't match reality

README.md lists typescript-sdk/ but actual directory is ts-sdk/. Also shows Phase 2/3 (Python/TypeScript) as unchecked todos when both are implemented in this PR.

11. CLAUDE.md embeds AI agent coordination protocol — development scaffolding leaked

CLAUDE.md:28-37 contains:

The coordinator server at http://localhost:4345 manages ../working.md...
- Post your update: curl -s -X POST http://localhost:4345/agent/sdk ...

This is development-time multi-agent tooling with a localhost server reference that won't exist in any environment. The CLAUDE.md itself acknowledges it should be removed ("I will remove this directive when we're done"). Remove before merging.

12. itoa reimplemented unnecessarily

go-sdk/types/base.go:42-62 hand-rolls an integer-to-string converter. strconv.Itoa from the Go standard library does this correctly. The "no third-party deps" rule doesn't restrict stdlib.

13. Python accessors are methods, not properties — inconsistent with test expectations

Tests call self.client.sessions (no parens), but sessions is defined as a regular method, not a @property. Accessing without () returns a bound method object, not a SessionAPI. Either add @property decorators to all accessors or fix the tests to use ().

14. No CI integration for the SDK

The ambient-sdk/Makefile is standalone with no hook into the root CI. Python, Go, and TypeScript tests will not run on PRs, allowing silent regressions.

🔵 Minor Issues

15. Empty go.sum files — correct for a stdlib-only module, but worth confirming intentional.

16. Non-idiomatic HTTP body construction in Go client

client.go:111,117-119: Request is created with nil body, then req.Body is set via io.NopCloser(strings.NewReader(string(body))). More idiomatic: pass bytes.NewReader(body) directly to http.NewRequestWithContext and avoid the []byte→string round-trip.

17. TypeScript client lacks URL/token format validation — Go and Python validate URLs (reject example.com, require http/https) and token formats. TypeScript only checks non-empty. Inconsistent security posture.

18. Tests assert on private attributes — test_client.py:79: assert client._base_url == "..." creates tight coupling. Prefer behavioral tests.

19. Python _request returns Any — Propagates Any through all API responses. Consider dict | None.

Positive Highlights

Generator architecture is excellent: OpenAPI → multi-language generation with hash-based spec tracking (Spec SHA256 in headers) makes drift detection easy and automatic.
Consistent security posture: Token redaction via sanitizeLogURL and slog.Debug-only logging in Go; URL placeholder rejection in both Go and Python; structured error surfaces that don't leak response bodies.
Generic Iterator[T] in Go: Clean pagination without loading all pages in memory.
errors.Join in Go builders: Collects all validation failures before returning — good UX.
Builder pattern with read/status field separation: SessionBuilder exposes only writable fields; SessionStatusPatchBuilder exposes only status fields. Prevents accidental spec-vs-status confusion.
TypeScript ambientFetch error handling: Field-by-field type narrowing instead of unsafe as casts.
Python context manager support: with AmbientClient(...) as client: for proper connection cleanup.
Zero external dependencies in Go: Right default for an SDK.
waitForPhase in the Go example: Practical, respects context.Context, handles terminal states correctly.

Recommendations

Priority 1 — must fix before merge:

Fix absolute dev path in Python generator template — relativize like Go template does
Remove duplicate Python SDK root-level files (keep only ambient_platform/ package)
Fix project_settingss → project_settings typo in Python client
Fix or remove test_client.py tests for nonexistent accessors (agents, tasks, etc.)
Add token validation to AmbientClient._validate_config() to match test expectations
Remove coordinator server protocol from CLAUDE.md (localhost:4345 reference)

Priority 2 — should fix before merge:
7. Fix README Go module path to match actual go.mod
8. Replace deprecated strings.Title in generator
9. Add @property decorators to Python client accessors OR fix tests to use ()
10. Wire SDK tests into root CI pipeline

Priority 3 — follow-up PR:
11. Replace custom itoa with strconv.Itoa
12. Add URL/token validation to TypeScript client for consistency
13. Implement SecureToken wrapper in Go or add redacting String() on Client

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-25T04:58:47Z

Claude Code Review

Summary

This PR introduces a substantial new components/ambient-sdk/ subtree: a code generator (Go text/template-based) plus generated Go, Python, and TypeScript client SDKs for the Ambient Platform public API. The architecture is clean — a single OpenAPI spec drives three language-idiomatic SDKs with consistent token security, builder patterns, and error hierarchies.

Overall the approach is solid, but there are several issues ranging from test/implementation drift (tests reference APIs that don't exist yet) to a leaked absolute local path in committed files, a method name typo, and silent parameter drops in the Go client.

Issues by Severity

⛔ Blocker Issues

1. Test suite references resources that don't exist in generated SDK

python-sdk/tests/test_client.py:TestClientResourceAccessors (lines 148-180) asserts .agents, .tasks, .skills, .workflows, .workflow_skills, .workflow_tasks, .permissions, .repository_refs, .project_keys accessors exist on AmbientClient. However, the generated ambient_platform/client.py only exposes projects(), project_settingss(), sessions(), users(). These tests will fail immediately.

test_client.py:149  self.client.agents is not None    # AttributeError
test_client.py:152  self.client.tasks is not None     # AttributeError
# ... 10 more failures

Action: Reconcile the test file with the current generated output OR regenerate the SDK from the complete OpenAPI spec.

🔴 Critical Issues

2. Absolute local developer path leaked in generated file header

python-sdk/ambient_platform/client.py:2 and _base.py:2 contain:

# Source: /home/mturansk/projects/src/github.com/ambient/platform/...

This exposes a contributor's home directory in committed, public-facing SDK code. The Go SDK correctly uses a relative path. Fix the Python generator template (http_client.py.tmpl, base.py.tmpl) to use a relative path.

3. project_settingss() method name typo (double 's')

python-sdk/ambient_platform/client.py:161 — the generator incorrectly pluralizes project_settings to project_settingss. The public API surface is broken. Fix the pluralization logic in http_client.py.tmpl.

4. Go doWithQuery silently drops OrderBy and Fields parameters

go-sdk/client/client.go:170-189 — ListOptions.OrderBy and .Fields are defined and exposed to callers, but doWithQuery never serializes them into query parameters. Callers who set OrderBy get silently ignored results.

5. Python _handle_response checks wrong JSON key

python-sdk/ambient_platform/client.py:135 checks "error" in error_data, but the API error schema uses "code" and "reason" (matching _base.py and the Go types.APIError). This condition is never true for well-formed API error responses — every error falls through to the generic HTTP path. Change to check "code" in error_data.

🟡 Major Issues

6. strings.Title is deprecated in generator

generator/main.go:301 (template funcmap) uses strings.Title, deprecated since Go 1.18. Replace with golang.org/x/text/cases. Will produce golangci-lint warnings and is a footgun for non-ASCII resource names.

7. CLAUDE.md commits localhost coordinator server protocol

CLAUDE.md:22-33 documents a development-time coordinator at http://localhost:4345 with curl POST instructions. This is a development-session artifact. It will confuse contributors and shouldn't be in committed code. Remove before merging.

8. Stale duplicate Python files at python-sdk/ root level

The PR includes python-sdk/client.py, python-sdk/project.py, python-sdk/session.py, etc. at the package root alongside the correct python-sdk/ambient_platform/ package. These appear to be an earlier generation pass. pyproject.toml won't package them, but they create confusion. Remove them.

9. Go/TypeScript validation inconsistency on example.com URLs

go-sdk/client/client.go:196 rejects example.com as a placeholder URL. ts-sdk/tests/client.test.ts:5 creates a client with baseUrl: 'https://api.example.com' and expects success — the TS constructor does not apply the same check. Inconsistent security posture across SDKs.

10. Python from_env() missing ANTHROPIC_VERTEX_PROJECT_ID fallback

Go SDK falls back to ANTHROPIC_VERTEX_PROJECT_ID when AMBIENT_PROJECT is unset; Python does not. Identical environment setups will succeed in Go but fail in Python.

11. No Go unit tests

Python and TypeScript both ship unit test suites. The Go SDK has only examples. Unit tests for client construction, URL validation, error parsing, and doWithQuery parameter forwarding are expected per project standards (go test ./...).

:blue_circle: Minor Issues

12. Inefficient body encoding in Go do()

client.go:117: io.NopCloser(strings.NewReader(string(body))) — body is already []byte. Use bytes.NewReader(body) and set req.ContentLength = int64(len(body)).

13. interface{} in Go 1.21 code — use any idiomatically.

14. APIError as frozen dataclass + Exception — err.args is always (), breaking pickling and some introspection tools. Not blocking, but worth a comment or a switch to a plain class.

15. No SDK integration in root Makefile or CI pipeline — SDKs won't be validated in PR checks without explicit CI steps.

Positive Highlights

Clean generator architecture: text/template-based generator with per-resource output and spec SHA256 stamps is maintainable and enables reproducibility checks.
Token security discipline: All three SDKs never log raw tokens. Go sanitizeLogURL and Python/TS [REDACTED] patterns follow project security standards correctly.
Builder pattern: SessionBuilder, ProjectBuilder, etc. across all three SDKs provides a clean, validated API surface that enforces required fields before HTTP calls.
Context-aware Go HTTP: http.NewRequestWithContext throughout is correct for timeout/cancellation propagation.
TypeScript type over interface: Consistent with project frontend standards.
slog structured logging: Modern, structured logging is appropriate for an SDK.
AbortSignal in TS: RequestOptions.signal enables proper cancellation from callers.
Comprehensive TS tests: Good coverage of construction, env-based creation, error formatting, and resource accessor properties.

Recommendations

Priority order before merge:

[Blocker] Reconcile test_client.py with actual generated SDK surface.
[Critical] Remove absolute path from Python generator templates.
[Critical] Fix project_settingss typo in http_client.py.tmpl pluralization logic.
[Critical] Forward OrderBy/Fields in Go doWithQuery (go/http_client.go.tmpl).
[Critical] Fix Python _handle_response to check "code" not "error".
[Major] Remove localhost coordinator section from CLAUDE.md.
[Major] Delete stale python-sdk/*.py root-level files.
[Major] Add Go unit tests.
[Major] Add SDK targets to CI pipeline.
[Minor] Replace deprecated strings.Title with golang.org/x/text/cases.

Generated with Claude Code

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

…nerator docs Key changes: - Add generated Python SDK with full CRUD client for all 4 resources - Regenerate Go and TypeScript SDKs with relative spec paths and corrected default URL (8080) - Update generator.md with TypeScript support, CI integration, and security documentation - Update production kustomization overlay

markturansky · 2026-02-26T12:45:36Z

Rebased. All feedback addressed to date.

github-actions · 2026-02-26T12:55:41Z

Claude Code Review

Summary

This PR introduces a new components/ambient-sdk/ component: a Go-based SDK generator driven by OpenAPI spec, and generated Go, Python, and TypeScript SDKs with CRUD and action support for sessions, projects, project settings, and users. The architecture (generator to spec to SDKs) is sound and the documentation is excellent. However, several blockers prevent the generated code from passing its own test suite, and there are structural bugs in the generator.

Issues by Severity

Blocker Issues

1. Generated Python client.py fails its own test suite

test_client.py tests features NOT implemented in the generated client.py. Pytest would fail:

test_short_token_raises: _validate_config() has no token length check
test_placeholder_token_raises: only URLs are checked for placeholder text, not tokens; test expects ValueError for YOUR_TOKEN_HERE
test_invalid_project_chars_raises: code only checks if not self._project; test expects ValueError for 'bad project!'
test_long_project_raises: no length check exists; test expects ValueError for 64-char project name
test_valid_client_creation line 81: assert client._base_path == /api/ambient-api-server/v1 but _base_path is not an attribute
Error message mismatches throughout (e.g., code raises 'base_url cannot be empty', test matches 'base URL cannot be empty')

The template http_client.py.tmpl is the source of truth and also lacks these validations. Fix the template then regenerate.

2. Missing @Property decorator on Python accessor methods

http_client.py.tmpl line 155 generates plain methods. Tests access them as properties (client.sessions not client.sessions()). Without @Property, client.sessions returns a bound method, not a SessionAPI. The test self.client.sessions is api will fail because each attribute access creates a distinct bound-method object in Python.

3. Generator pluralization bug: project_settingss double-s

generator/model.go lines 208-209 appends 'es' when a name ends in 's'. ProjectSettings snake_cases to project_settings (ends in 's'), yielding project_settingses via the template. The committed generated code shows project_settingss (simple +s), meaning the committed output was built from a different generator version. Either way both are wrong API names. The generator needs an exceptions map for already-plural words.

Critical Issues

4. Template/generated code divergence

The committed generated files were not produced by the templates in this PR:

client.py line 2 has a developer-local absolute path: Source: /home/mturansk/projects/src/...
_handle_response in client.py checks 'error' key; http_client.py.tmpl checks 'code' -- they match different API error shapes
TypeScript SDK exists but has no Makefile target -- cannot be reproduced from source

Run make generate-sdk and commit freshly generated output before merging.

5. TypeScript SDK excluded from Makefile

Makefile lines 11-21 only generate Go and Python. The TS SDK is hand-managed with no regeneration path. If the OpenAPI spec changes, the TS SDK will silently drift.

Major Issues

6. _base_path should be a named attribute -- tests expect client._base_path == /api/ambient-api-server/v1 on line 81 of test_client.py. Extract as a class constant in the template.

7. pluralize() fragile for irregular plurals -- model.go:206-215 uses simplistic suffix-replacement. Any future resource ending in 's' produces wrong accessor names. Consider github.com/gertd/go-pluralize or an explicit exceptions map.

8. toGoName() can panic on empty rune slice (model.go:54) -- a leading underscore in an OpenAPI field name produces an empty part after splitting, causing runes[0] to panic. Add: if len(runes) == 0 { continue }

9. Silent schema parsing failures in parser.go -- bare ok checks with silent continue (~lines 195, 214, 284) swallow schema errors. Convert to log.Printf WARN or return explicit errors.

10. Token validation documented but not implemented -- authentication.md specifies sha256~ prefix, JWT dot-structure, GitHub ghp_/gho_/ghu_/ghs_ prefixes, 20-char minimum. None is implemented in any http_client.*.tmpl template. Add validation to all three language templates.

Minor Issues

No retry/backoff logic (acceptable for MVP -- document it)
list_all() can loop indefinitely on concurrent deletes: prefer terminating when len(result.items) < size
/api/ambient-api-server/v1 hardcoded as literal in all three SDKs; expose as a named constant
Error key mismatch: template checks 'code' in error JSON; generated client.py checks 'error'
test_project_settingss_accessor test name leaks the generator bug; fix both together

Positive Highlights

Generator architecture (OpenAPI spec to Go templates to multi-language SDKs) is well-structured and extensible
Documentation (README.md, CLAUDE.md, docs/authentication.md, docs/architecture.md) is thorough and well-written
APIError has identical shape and message format across all three languages
Go SDK Iterator[T] with generics and Err() accessor follows idiomatic Go conventions
TypeScript SDK correctly avoids any types throughout
Go HTTP client uses http.NewRequestWithContext for correct cancellation threading
sanitizeLogURL() with regex redactor prevents tokens from appearing in logs (matches project security standards)

Recommendations (priority order before merge)

Add @Property to all accessor methods in http_client.py.tmpl
Fix pluralize() for already-plural words (ProjectSettings -> project_settings, no suffix)
Implement token and project name validation in all three http_client.*.tmpl templates per authentication.md
Align error messages in http_client.py.tmpl with test_client.py assertions
Add _base_path as a class constant in the Python template
Add TypeScript SDK generation to the Makefile
Run make generate-sdk and commit freshly generated output (resolves local-path issue and template/output divergence)
Add empty-slice guard before runes[0] in model.go:toGoName()

Generated with Claude Code

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-26T14:36:41Z

Claude Code Review

Summary

PR 677 introduces a new components/ambient-sdk component: a code generator (Go) that reads an OpenAPI spec and emits client libraries in Go, Python, and TypeScript. The security fundamentals are strong (dual-layer token redaction in Go, input validation, no K8s dependencies in the SDK) but there are several gaps between the template/generated code, inconsistent documentation, a leaked internal environment variable name, and a missing TypeScript flag in the Makefile that should be addressed before merge.

Issues by Severity

Blocker Issues

1. Generated client.go is out of sync with the generator template

generator/templates/go/http_client.go.tmpl includes OrderBy and Fields query parameter handling in doWithQuery() (template lines 195-199), but go-sdk/client/client.go (generated 2026-02-24) is missing both parameters entirely. Running make generate-sdk would produce a different file than what is committed. Either regenerate after template finalization, or lock the template to match the committed output before merging.

2. Makefile generate-sdk target is missing --ts-out

The generator binary accepts a -ts-out flag (confirmed in generator/main.go) and the TypeScript SDK exists in ts-sdk/, but the Makefile only passes -go-out and -python-out. Additionally, verify-sdk only compiles Go and does a Python import check -- it never runs npm run build or tsc for TypeScript. After the first API spec update, the TypeScript SDK will silently diverge.

Critical Issues

3. ANTHROPIC_VERTEX_PROJECT_ID leaks an internal implementation concept into a public SDK

go-sdk/client/client.go:99 and the generator template both fall back to ANTHROPIC_VERTEX_PROJECT_ID as an alias for AMBIENT_PROJECT, and this internal env var name appears directly in the error message returned to SDK users. The same pattern appears in go-sdk/examples/main.go:32. External users have no context for what ANTHROPIC_VERTEX_PROJECT_ID means. Remove this fallback or restrict it to strictly internal usage.

4. README.md is severely stale and contradicts the actual implementation

Phase 2 (TypeScript SDK) is marked TODO -- but the TypeScript SDK is in this PR
Phase 3 (Python SDK) is marked TODO with Generate Pydantic models -- but the Python SDK uses dataclasses, explicitly avoiding Pydantic
Directory structure shows typescript-sdk/ (future) -- actual directory is ts-sdk/
Getting Started shows wrong module path github.com/ambient/platform-sdk
Quick Start imports github.com/ambient/platform-sdk/client -- actual module is github.com/ambient-code/platform/components/ambient-sdk/go-sdk

This README will immediately mislead first-time SDK users.

Major Issues

5. CLAUDE.md references SecureToken type that does not exist in generated code

components/ambient-sdk/CLAUDE.md states that SecureToken wraps tokens and implements slog.LogValuer, and that sanitizeLogAttrs provides defense-in-depth log sanitization. Neither SecureToken nor sanitizeLogAttrs appear in go-sdk/client/client.go -- the token is stored as a plain string and only sanitizeLogURL() is present. CLAUDE.md describes aspirational security architecture that was not implemented. Update CLAUDE.md to match the actual implementation, or implement the described types.

6. Production kustomization change lacks PR description context

components/manifests/overlays/production/kustomization.yaml has a +1/-1 change with no mention in the PR description. Production manifest changes should always be explicitly described to support rollback decisions and audit trails.

7. SDK ships with a known-broken end-to-end smoke test

CLAUDE.md documents: It currently returns 404 because the API server has not been migrated to serve /api/ambient-api-server/v1/sessions yet. This should be captured as a follow-up issue so it does not silently persist.

8. io.NopCloser(strings.NewReader(string(body))) is unnecessarily verbose

go-sdk/client/client.go:117 converts []byte body to string, wraps in strings.NewReader, then wraps in io.NopCloser. More idiomatic: pass bytes.NewReader(body) directly to http.NewRequestWithContext. Since this is in the generator template, fixing it propagates to all generated clients.

Minor Issues

9. sanitizeLogURL() will never match tokens in practice

The function is called with the bare URL -- tokens are passed in headers, not query strings. The regex is correct but will never trigger on a URL.

10. go.sum files are empty -- verify go mod tidy was run

Both go-sdk/go.sum (0 lines) and examples/go.sum (0 lines) are committed empty. For the examples module which depends on go-sdk via a replace directive, confirm go mod tidy was run and the replace directive is in examples/go.mod.

11. test.sh relationship to pytest-based tests is undocumented

python-sdk/test.sh (231 lines) coexists with pytest-based unit tests in python-sdk/tests/ with no documentation of when to run which or which is required for CI.

12. docs/architecture.md references ../working.md as a coordination protocol

This appears to be a session-level coordination artifact, not a committed project file. Linking it from architecture docs that external contributors will read is confusing.

Positive Highlights

Strong security posture: Dual-layer URL sanitization in the Go SDK, token placeholder detection (rejects YOUR_TOKEN_HERE, password, demo, etc.), minimum length and format validation on client construction -- well above baseline.
Zero external dependencies: Go SDK uses only stdlib; Python SDK uses only httpx. Makes auditing and embedding trivial.
Comprehensive test coverage: All three SDK languages have test suites -- Python has 759-line type tests and 197-line client tests; TypeScript has base, client, and types test files.
Builder pattern across all three languages: Fluent, idiomatic APIs with Validate() / validate() checks before HTTP calls.
Code generation approach: OpenAPI-spec-driven generator is the right architecture for keeping three language SDKs in sync long-term.
docs/authentication.md: Exceptionally clear token format documentation with concrete examples for OpenShift SHA256, JWT, and GitHub token formats.
Context-manager support in Python: with AmbientClient(...) as client: is exactly what Python users expect.
ListOptions with max-size enforcement: The 65500-item cap in _base.py prevents accidental unbounded list operations.

Recommendations

Before merge (in priority order):

Regenerate all SDKs by running make generate-sdk with the finalized templates and commit the result, or align the template with what is committed (Blocker 1)
Add -ts-out ts-sdk and TypeScript verification (npm run build) to the Makefile (Blocker 2)
Remove ANTHROPIC_VERTEX_PROJECT_ID from the public SDK surface (Critical 3)
Rewrite README.md to reflect the actual shipped state of all three SDKs (Critical 4)
Update CLAUDE.md to remove references to SecureToken/sanitizeLogAttrs or implement them (Major 5)
Add a one-sentence description of the kustomization change to the PR description (Major 6)

Follow-up issues to file:

Tracking issue for API server migration (so the known-404 smoke test has a linked resolution path)
Replace io.NopCloser(strings.NewReader(string(body))) with bytes.NewReader(body) in the generator template

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

github-actions · 2026-02-26T15:35:22Z

Claude Code Review

Summary

This PR introduces a significant new component: a multi-language SDK generator plus generated Go, Python, and TypeScript client libraries for the Ambient Platform public API. The overall architecture is clean — a Go-based code generator reads OpenAPI specs and produces idiomatic clients in all three languages. Token security is handled well across the board, and the builder patterns are ergonomic. However, there are two failing tests and one compile error in the Go examples that need to be fixed before merge.

Issues by Severity

🚫 Blocker Issues

1. Python test test_placeholder_domain_raises will FAIL
File: components/ambient-sdk/python-sdk/tests/test_client.py:65-71

The actual error raised by _validate_config is "Placeholder URLs are not allowed" (client.py:82). The match= argument uses re.search() (case-sensitive), so match="placeholder domain" will never match that string. Fix: change to match="[Pp]laceholder" or match="not allowed".

2. Python test typo causes AttributeError
File: components/ambient-sdk/python-sdk/tests/test_client.py:152

assert self.client.project_settingss is not None  # double 's'

The property is project_settings. This test fails at runtime with AttributeError.

3. Go examples won't compile — Interactive field missing from generated types
File: components/ambient-sdk/go-sdk/examples/main.go:87,104

examples/main.go calls .Interactive(true) and reads createdSession.Interactive, but the generated Session struct (go-sdk/types/session.go) has no Interactive field and SessionBuilder has no .Interactive(bool) method. The verify-sdk Makefile target only runs go build ./... on the go-sdk root, so the examples subtree is silently skipped — misleading for users who try go run examples/main.go.

Fix: either add interactive to the OpenAPI spec so the generator picks it up, or remove the field from the example until it is.

🔴 Critical Issues

4. sanitizeLogURL recompiles regexp on every HTTP request
File: components/ambient-sdk/go-sdk/client/client.go:230 (and template http_client.go.tmpl:229)

regexp.MustCompile(...) is called inside the function on every invocation. Move it to a package-level var — this is hit on every debug-logged HTTP request.

5. Validation inconsistency across SDKs — project name charset

SDK	Length check	Charset `[a-z0-9-]`
Go	✅ >63 rejected	❌ Missing
Python	✅ >63 rejected	✅ Enforced
TypeScript	❌ Only non-empty	❌ Missing

Go checks length but not the character set that Python enforces. TypeScript validates nothing beyond non-empty. Users switching SDKs will hit surprising failures.

🟡 Major Issues

6. TypeScript client has no URL validation
File: components/ambient-sdk/ts-sdk/src/client.ts

Go and Python reject example.com and non-HTTP/S schemes. TypeScript silently accepts any string. The TypeScript tests use https://api.example.com as a "valid" URL (client.test.ts:6), which both Go and Python would reject — inconsistent cross-SDK behaviour.

7. strings.Title is deprecated — will fail golangci-lint
File: components/ambient-sdk/generator/main.go:306

strings.Title has been deprecated since Go 1.18. Needs replacement with golang.org/x/text/cases or a local helper to pass the pre-commit lint hook.

8. Hardcoded resource and action lists in parser
File: components/ambient-sdk/generator/parser.go:40-44,348

Resources (Session, User, Project, ProjectSettings) and actions (start, stop) are hardcoded. Adding a new resource or action requires editing the generator source. At minimum, document this constraint clearly in generator.md.

9. APIError as a frozen dataclass inheriting Exception is an anti-pattern
File: components/ambient-sdk/python-sdk/ambient_platform/_base.py:53

Frozen dataclasses block object.__setattr__, so Exception.__init__ — which sets self.args — is never called. The custom __str__ papers over this, but e.args, raise e from another, and traceback.format_exception will behave unexpectedly. Consider a regular (non-frozen) dataclass or a plain class.

🔵 Minor Issues

10. Go Sessions()/Projects() return a new struct on every call (session_api.go:22-24) while Python caches lazily. Minor allocation overhead in tight loops.

11. CLAUDE.md directory listing references exceptions.py and types.py but the actual package uses _base.py and per-resource files — outdated.

12. from_env(**kwargs: Any) in Python client passes untyped kwargs to the constructor — a misspelled keyword arg (e.g. timout=30.0) is silently swallowed.

13. pluralize("urls") returns "urlses" (ends in s so appends es). Fragile for future resources.

Positive Highlights

Token security across all three SDKs: Raw tokens are never logged. Go has sanitizeLogURL, Python/TypeScript never expose the token value in logs.
Spec SHA256 hash in generated file headers: Makes drift detection trivial.
Go builder pattern with validation: NewSessionBuilder().Name("...").Build() returns (*Session, error) — idiomatic, prevents partially-constructed objects.
Python context manager support: with AmbientClient(...) as c: follows Python best practices.
AbortSignal support in TypeScript: RequestOptions.signal enables proper fetch cancellation.
ListAll/listAll pagination iterators: Automatic pagination is a significant DX win.
Structured logging via slog: Standard library since Go 1.21 — zero extra deps.
Consistent env var names: AMBIENT_TOKEN, AMBIENT_PROJECT, AMBIENT_API_URL are identical across all three SDKs.
Input validation at construction time: Token length, placeholder detection, and URL scheme checks catch misconfiguration early.

Recommendations

Priority order before merge:

Fix the two failing Python tests (blockers Outcome: Reduce Refinement Time with agent System #1, Epic: RAT Architecture & Design #2) — trivial string fixes
Fix or remove the non-compiling Interactive usage in Go examples (Epic: Data Source Integration #3)
Move regexp.MustCompile to package-level in template (Epic: AI Agent Development #4) — one-line fix
Align project name charset validation across all three SDKs (Epic: Jira Integration & Workflow #5)
Fix deprecated strings.Title (Test: Automation Workflow Validation #7) — required to pass golangci-lint pre-commit hook
Document the hardcoded resource list limitation (Test: Updated Workflow Validation #8) — low effort, high clarity

Review by Claude Code (claude-sonnet-4-6)

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

…nd bug fixes Key changes: - Replace deprecated strings.Title with safe rune-based title function in generator - Replace custom itoa() with strconv.Itoa() in Go SDK - Add input validation to TS SDK (token length, placeholder, project length, URL scheme) - Fix URL construction in Python SDK (urljoin → string concat) - Allow underscores in project names across SDKs - Remove Interactive field from session - Fix struct field alignment and trailing whitespace in Go SDK - Simplify Python test suite

github-actions · 2026-02-26T16:08:41Z

Claude Code Review

Summary

PR #677 introduces a code-generated multi-language SDK (Go, Python, TypeScript) for the Ambient Platform's public REST API. The design is solid — clean builder pattern, good token security posture, and proper API/query layer separation. However, there are a few issues ranging from a committed binary (blocker) to a nil-dereference bug and a failing test that should be addressed before merge.

Issues by Severity

🚫 Blocker Issues

1. Compiled binary committed to git — generator/ambient-sdk-generator

The PR includes a 5.3MB ELF x86-64 executable committed directly into the repository (components/ambient-sdk/generator/ambient-sdk-generator). The PR diff entry shows "additions":0,"deletions":0 because git tracks it as a binary blob.

Binaries must not be committed to source control — they bloat the repo, cannot be code-reviewed, and are a supply chain security risk
The .gitignore at the root of the SDK component does not exclude it
CI should make build-generator from source; consumers can build locally with the same command

Fix: Add generator/ambient-sdk-generator and bin/ to .gitignore, remove the binary from the PR.

🔴 Critical Issues

2. Nil pointer dereference in all ListAll() implementations (Go SDK)

Every resource's ListAll method dereferences the opts pointer unconditionally:

// go-sdk/client/session_api.go:95
func (a *SessionAPI) ListAll(ctx context.Context, opts *types.ListOptions) *Iterator[types.Session] {
    return NewIterator(func(page int) (*types.SessionList, error) {
        o := *opts  // ⚠️ PANIC if opts is nil
        o.Page = page
        return a.List(ctx, &o)
    })
}

A caller using client.Sessions().ListAll(ctx, nil) will panic at runtime. Same pattern exists in project_api.go, project_settings_api.go, and user_api.go.

Fix: Guard against nil opts:

var o types.ListOptions
if opts \!= nil {
    o = *opts
}
o.Page = page

3. Failing test — test_project_settingss_accessor typo (Python)

components/ambient-sdk/python-sdk/tests/test_client.py:151-153 accesses a non-existent attribute:

def test_project_settingss_accessor(self):
    assert self.client.project_settingss is not None  # double 's' — AttributeError

The correct attribute is project_settings. This test will fail with AttributeError at runtime. The fact that it exists as-is suggests the Python test suite isn't being run in CI for this branch.

Fix: Rename to project_settings. This should also prompt verifying that pytest is gated in CI.

🟡 Major Issues

4. Generator hardcodes spec source path, ignoring the -spec flag for the generated header

generator/main.go:41:

// Uses -spec flag for parsing, but overrides it with a hardcoded path for the generated comment
relativeSpecPath := "../../ambient-api-server/openapi/openapi.yaml"
header := GeneratedHeader{
    SpecPath:  relativeSpecPath,  // always this, regardless of -spec
    ...
}

The -spec flag is used correctly to parse the spec, but the generated header comment (// Source: ...) always shows the hardcoded path. This misleads readers about where the spec actually came from when generating from a non-standard path.

Fix: Derive relativeSpecPath from the actual *specPath argument or use the flag value directly.

5. Generated timestamps cause unnecessary churn in version control

Every generated file includes:

// Generated: 2026-02-26T15:57:52Z

Re-running the generator with an unchanged spec will modify every generated file's timestamp, creating meaningless diffs and making git blame harder to read. The spec SHA256 hash already provides the meaningful provenance signal.

Fix: Remove the Timestamp field from GeneratedHeader, or make it opt-in via a flag.

6. Inconsistent URL validation across SDKs — TS SDK missing placeholder domain check

Go and Python SDKs both reject placeholder domains:

// Go
if strings.Contains(rawURL, "example.com") || strings.Contains(rawURL, "placeholder") { ... }

# Python  
if "example.com" in self._base_url or "placeholder" in self._base_url: ...

The TypeScript AmbientClient constructor validates URL scheme only — no placeholder check. The TS tests even use https://api.example.com freely. This inconsistency will confuse SDK consumers expecting uniform behavior.

Fix: Add placeholder domain validation to the TS AmbientClient constructor (or explicitly document the intentional difference, and use a non-example.com domain in TS tests).

7. Python _request parameter shadows import json module

client.py:106:

def _request(
    self,
    method: str,
    path: str,
    *,
    json: Optional[dict[str, Any]] = None,   # shadows the 'import json' module
    ...
) -> Any:

Within _request's scope, json refers to the parameter, not the stdlib module. This currently works because json.JSONDecodeError is referenced in _handle_response (a separate method), but it's a maintenance hazard. Also, Union is imported but unused in this file (F401).

Fix: Rename the parameter (e.g., body or json_body) and remove the unused Union import.

🔵 Minor Issues

8. Production kustomization change included in SDK PR

components/manifests/overlays/production/kustomization.yaml has a +1/-1 change. Production manifest changes mixed into a feature PR deserve explicit attention. Please add a comment in the PR description explaining what this change does (image tag update? new service entry?).

9. README.md missing newline at end of file

components/ambient-sdk/README.md ends without a trailing newline (the pre-commit end-of-file-fixer hook should have caught this).

10. Python PEP 8: missing blank lines before property definitions

client.py:172-178 — the @property methods immediately follow __exit__ without a blank line separator, violating PEP 8 E302. While black enforces formatting, it does not enforce all PEP 8 spacing rules.

11. sanitizeLogURL in Go SDK sanitizes Bearer tokens from URLs (wrong direction)

client.go:229-232 — the function replaces Bearer xxx patterns inside URL strings. Bearer tokens should only appear in Authorization headers, never in URLs. This function will never fire in practice. Consider removing it or renaming it to clarify its purpose is a defense-in-depth measure for mis-encoded requests.

Positive Highlights

Excellent token security posture — dual-layer log redaction in Go (slog.LogValuer + ReplaceAttr), format validation on construction, and placeholder detection across all SDKs. Aligns perfectly with project security standards.
Builder pattern is ergonomic — NewSessionBuilder().Name("x").Prompt("y").Build() is clean and validates required fields before any network call.
Zero unnecessary dependencies — Go SDK uses only stdlib; Python uses only httpx. Keeps the SDK lightweight for external consumers.
NewClientFromEnv / from_env() / AmbientClient.fromEnv() — consistent env-based factory across all three SDKs, exactly the right UX for CI/CD environments.
URL path escaping — url.PathEscape(id) used correctly in Go for all ID path parameters.
Context-manager support in Python — with AmbientClient(...) as client: properly manages httpx.Client lifecycle.
Spec-hash in generated file headers — using the SHA256 of the spec as provenance is a good practice for tracking drift.
Good test coverage for validation paths — the TS and Python test suites thoroughly test constructor validation, fromEnv behavior, and API accessor caching.

Recommendations

🚫 Remove the committed binary before merge — add to .gitignore and document make build-generator in the README.
🔴 Fix nil-dereference in all ListAll methods — guard opts before dereferencing.
🔴 Fix the product_settingss test typo and verify pytest runs cleanly in CI.
🟡 Remove generated timestamps from file headers to reduce VCS noise.
🟡 Fix hardcoded spec path in generator header metadata.
🟡 Harmonize TS URL validation with Go/Python (or document the difference).
🟡 Rename json parameter in Python _request and remove unused Union import.
🔵 Add a comment to the PR explaining the production kustomization change.

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

…o, harden test inputs

github-actions · 2026-02-26T16:30:04Z

Claude Code Review

Summary

This PR introduces a substantial new component: a code generator and three SDK implementations (Go, Python, TypeScript) for the Ambient Platform public API. The overall design is solid — consistent builder patterns, input validation across all three SDKs, and a clean OpenAPI-driven generator. The SDK is correctly scoped as an external client library with no Kubernetes dependencies, matching its stated intent.

Key facts: 14,763 lines added across 88 files. Only 1 line deleted (the kustomization.yaml change).

Issues by Severity

Blocker Issues

None. This is additive — it does not touch existing platform components and failing SDKs will not break the running system.

Critical Issues

1. Nil pointer dereference in Go SDK ListAll

File: components/ambient-sdk/go-sdk/client/session_api.go:93-99

The opts parameter is a pointer with no nil guard. Callers passing nil will get a runtime panic inside the iterator's first Next() call (not at construction), making it hard to debug. The same pattern applies to all other resource ListAll methods generated from the template.

Concrete example: o := *opts panics if opts is nil.

Fix: Add a nil check before the copy: if opts == nil { opts = &types.ListOptions{} }

2. TypeScript SDK allows example.com as baseURL; test validates this behavior

File: components/ambient-sdk/ts-sdk/tests/client.test.ts:41-48

The test 'strips trailing slashes from baseUrl' uses https://api.example.com/// and expects the client to be created successfully. The Go SDK (validateURL) and Python SDK (_validate_config) both reject URLs containing example.com. The TypeScript AmbientClient constructor has no equivalent guard. This inconsistency is a correctness issue — the test accidentally documents it as expected behavior.

Major Issues

3. Parameter name json shadows the standard library import in Python client

File: components/ambient-sdk/python-sdk/ambient_platform/client.py:105-113

The _request method has a parameter named json which shadows the json module imported at the top of the file. While httpx handles JSON serialization internally (so the shadowed module is not invoked inside _request), mypy strict mode should catch this and the naming creates unnecessary confusion. A name like body or json_body avoids the collision. The fix belongs in http_client.py.tmpl.

4. Page size cap is inconsistent across SDKs

Python _base.py:88: caps at 65500
TypeScript base.ts:59: Math.min(opts.size, 65500)
Go doWithQuery: no size cap at all

The magic number 65500 is undocumented and only enforced in two of three SDKs. The Go SDK silently allows unbounded page sizes. Either document why this cap exists and enforce it consistently, or remove it if the API server enforces its own limit.

5. Generator computeSpecHash has a hardcoded file list

File: components/ambient-sdk/generator/main.go:357-364

The function hardcodes exactly four OpenAPI sub-spec filenames. When new OpenAPI resource files are added, the hash will not include them, causing generated files to appear up-to-date when they are stale. The file list should be derived from the resourceFiles map in parser.go or auto-detected by globbing openapi.*.yaml.

6. Token format validation described in CLAUDE.md is not implemented

File: components/ambient-sdk/CLAUDE.md:92

The CLAUDE.md states token format validation covers OpenShift sha256~, JWT (3 dot-separated base64 parts), and GitHub ghp_/gho_/ghu_/ghs_ prefixes. All three SDKs only check len(token) >= 20 and reject literal placeholder strings. Either implement the format validation (catches misconfiguration early) or remove the claim from CLAUDE.md to avoid misleading future contributors.

7. PEP 8 violation in generated Python — missing blank lines before @Property

File: components/ambient-sdk/python-sdk/ambient_platform/client.py:171-173

PEP 8 requires two blank lines between method definitions at class level. There is no blank line between exit and the first @Property. Since client.py is generated, the fix belongs in http_client.py.tmpl.

Minor Issues

8. Three identical template data structs

File: components/ambient-sdk/generator/main.go:82-98

goTemplateData, pythonTemplateData, and tsTemplateData are structurally identical. A single templateData struct would simplify the generator and reduce future drift.

9. resourceFiles map iteration order is non-deterministic

File: components/ambient-sdk/generator/parser.go:40-54

Go maps randomize iteration order. Resources are sorted by name afterward so the final output is deterministic, but error messages during the resource loop may appear in arbitrary order. Cosmetic, but worth noting.

10. ListOptions.to_params silently drops explicitly-set falsy values

File: components/ambient-sdk/python-sdk/ambient_platform/_base.py:106

The comprehension {k: v for k, v in self._params.items() if v} will silently omit page=0 because 0 is falsy. The default initializes page=1 so this is unlikely in practice, but if v is not None would be more correct semantics.

11. Broken kustomization.yaml target indentation (pre-existing, touched by PR)

File: components/manifests/overlays/production/kustomization.yaml:38-44

The target: block immediately after the commented-out postgresql-json-patch.yaml path appears intended for that patch but is parsed as the target for unleash-init-db-patch.yaml. Since this file was touched in this PR it is worth confirming whether this matches the intended behavior.

12. Go SDK generator writes partial output on template error

File: components/ambient-sdk/generator/main.go:343-351

If tmpl.Execute(f, data) fails midway through, a partially-written file remains on disk. Writing to a bytes.Buffer first and then flushing to disk atomically would prevent confusing half-generated files. Low risk for a developer tool.

Positive Highlights

Consistent validation across all three SDKs — token length, placeholder detection, URL scheme validation, and project name constraints are applied uniformly at construction time, failing fast before any network call.
Token security is handled correctly — the Go SDK sanitizes URLs before logging via sanitizeLogURL, and neither Python nor TypeScript log request URLs or headers. The token is never exposed in error messages.
Builder pattern prevents invalid state — SessionBuilder, ProjectBuilder, etc. validate required fields in Build() and expose only writable fields, making read-only API fields (phase, kube_cr_name, etc.) correctly inaccessible to callers.
SHA256 spec hash in generated file headers — tracking the hash of the OpenAPI spec at generation time makes staleness visible and auditable.
Generic iterator in Go SDK — the Iterator[T] with Listable[T] interface is idiomatic Go generics and correctly abstracts pagination across all resource types.
Context manager support in Python — enter/exit enables clean resource management and is the expected Python idiom for HTTP clients.
Strong test coverage for validation logic — both Python (test_client.py) and TypeScript (client.test.ts) thoroughly cover all constructor validation paths and error messages.
DO NOT EDIT headers on generated files — clearly marks the generated/source boundary, preventing manual edits from being silently overwritten.

Recommendations

Prioritized action items:

Fix ListAll nil panic (Critical) — One-line nil guard before the o := *opts copy in all ListAll implementations.
Align TypeScript example.com rejection (Critical) — Add the same URL guard the Go/Python SDKs have; update the test to use a non-example.com URL.
Fix Python json parameter shadowing (Major) — Rename to json_body in _request; update the template.
Standardize the size cap (Major) — Document the 65500 limit, add it to Go, or remove it from all.
Fix PEP 8 blank lines in Python template (Major) — Add blank line before @Property decorators in http_client.py.tmpl.
Update computeSpecHash to auto-detect spec files (Major) — Glob openapi.*.yaml or derive from resourceFiles map.
Reconcile CLAUDE.md token format validation claim (Major) — Implement or remove.

Generated with Claude Code

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

CLAUDE.md - Master project instructions, development standards
backend-development.md - Go backend, K8s integration patterns
frontend-development.md - NextJS, Shadcn UI, React Query patterns
security-standards.md - Auth, RBAC, token handling
k8s-client-usage.md - User token vs service account patterns
error-handling.md - Consistent error patterns
react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

markturansky mentioned this pull request Feb 24, 2026

WIP: Feat/Adds ambient-sdk #638

Closed

markturansky force-pushed the feat/ambient-sdk3 branch from df7b1d3 to fe6d570 Compare February 24, 2026 18:09

This comment has been minimized.

Sign in to view

markturansky force-pushed the feat/ambient-sdk3 branch from acaaa17 to 21be30f Compare February 25, 2026 04:52

markturansky added 5 commits February 26, 2026 07:35

feat: adds ambient-sdk w/ python,go,typescript sdks

c5af456

addresses PR feedback

5736ebd

PR feedback. Fixes issues. working examples/main.go

3e64cad

responses to latest feedback, ready to squash

cca1084

markturansky force-pushed the feat/ambient-sdk3 branch from 21be30f to cca1084 Compare February 26, 2026 12:45

last feedback

5b75c11

latest feedback, squash bait

ecfd16f

fix(sdk): clean up tests and examples — remove unused import, fix typ…

422703b

…o, harden test inputs

maskarb approved these changes Feb 26, 2026

View reviewed changes

jeremyeder merged commit b50d2b2 into ambient-code:main Feb 26, 2026
15 checks passed

Conversation

markturansky commented Feb 24, 2026

Uh oh!

This comment has been minimized.

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

🚫 Blocker Issues

🔴 Critical Issues

🟡 Major Issues

🔵 Minor Issues

Positive Highlights

Recommendations

What Amber Loaded for Code Review

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

🚫 Blocker Issues

🔴 Critical Issues

🟡 Major Issues

🔵 Minor Issues

Positive Highlights

Recommendations

What Amber Loaded for Code Review

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

🚫 Blocker Issues

🔴 Critical Issues

🟡 Major Issues

🔵 Minor Issues

Positive Highlights

Recommendations

What Amber Loaded for Code Review

Uh oh!

github-actions bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

🚫 Blocker Issues

🔴 Critical Issues

🟡 Major Issues

🔵 Minor Issues

Positive Highlights

Recommendations

What Amber Loaded for Code Review

Uh oh!

github-actions bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

⛔ Blocker Issues

🔴 Critical Issues

🟡 Major Issues

:blue_circle: Minor Issues

Positive Highlights

Recommendations

What Amber Loaded for Code Review

Uh oh!

markturansky commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Issues by Severity

Blocker Issues

Critical Issues

Major Issues

Minor Issues

github-actions bot commented Feb 24, 2026 •

edited

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

github-actions bot commented Feb 25, 2026 •

edited

Loading

github-actions bot commented Feb 25, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading

github-actions bot commented Feb 26, 2026 •

edited

Loading