feat(sdk): gRPC session watch, base path fix, and CLI module sync

[markturansky]

Summary

gRPC Session Watch

• Add SessionWatcher for real-time session event streaming via gRPC
• Add types/session_watch.go with SessionWatchEvent type and IsCreated/IsUpdated/IsDeleted helpers
• Add examples/watch/main.go demonstrating watch usage
• Add google.golang.org/grpc and ambient-api-server proto dependencies

Review Fixes

• Fix defer cancel() blocker: timeoutCancel now stored on SessionWatcher, called in Stop()
• Replace deprecated grpc.DialContext/WithBlock with grpc.NewClient
• Replace fragile string trimming in deriveGRPCAddress with url.Parse + net.JoinHostPort
• Extract hardcoded gRPC port to grpcDefaultPort constant

SDK Generator: Base Path from Spec

• Generator now derives base path (e.g. /api/ambient/v1) from OpenAPI spec path keys instead of hardcoding /api/ambient-api-server/v1
• Updates model.go, parser.go, and all three language templates (Go, Python, TypeScript)
• Regenerates all SDK output with the correct /api/ambient/v1 base path

CLI Module Sync

• Run go mod tidy on ambient-cli to sync after SDK go version bump (1.21 → 1.24.0)

Test plan

• go build ./... passes in go-sdk/
• go build ./... passes in ambient-cli/
• go vet ./... clean
• go fmt ./... clean
• Watch example connects and streams session events against a live cluster
• All SDK URLs now use /api/ambient/v1

🤖 Generated with Claude Code

[github-actions]

Summary

PR adds real-time gRPC session watch support to the Go SDK via a new SessionWatcher type, companion event types, and a usage example. The implementation is well-structured with clean channel-based event streaming and proper resource cleanup via Stop(). However, one critical dead-API issue and several other concerns should be addressed before merge.

Issues by Severity

Blocker Issues

None.

Critical Issues

1. WatchOptions.ResourceVersion is declared but silently ignored

• File: components/ambient-sdk/go-sdk/client/session_watch.go (Watch function)
• Problem: WatchOptions.ResourceVersion is a documented field but is never passed to the gRPC request — it is quietly dropped. Callers who set ResourceVersion to resume a watch from a known point will get silent, incorrect behavior — always receiving from latest. This is a broken contract.
• Violates: error-handling.md (silent failures)
• Fix: Either wire the field through to WatchSessionsRequest if the proto supports it, or remove it from WatchOptions until implemented. Do not export dead API surface.

Major Issues

2. context.Context stored in struct field

• File: components/ambient-sdk/go-sdk/client/session_watch.go (SessionWatcher struct)
• Problem: SessionWatcher stores ctx context.Context as a struct field. The Go docs explicitly advise against this: "Do not store Contexts inside a struct type." It couples the watcher lifetime to a specific context instance and makes per-call propagation harder to reason about.
• Note: The cancel func as a field is acceptable for lifecycle types. Document the rationale if keeping ctx in the struct.

3. Missing tests for new functionality

• Files: client/session_watch.go, types/session_watch.go
• Problem: No _test.go files included. The PR test plan only covers build/vet/fmt. No behavioral coverage for convertEvent, convertSession, deriveGRPCAddress, or SessionWatchEvent helper methods.
• Suggestion: At minimum, unit tests for deriveGRPCAddress() (various URL forms, missing port, invalid URL), convertEvent() (nil input, all event types, unknown type), and SessionWatchEvent.IsCreated/IsUpdated/IsDeleted().

4. UNKNOWN event type propagated to consumers

• File: components/ambient-sdk/go-sdk/client/session_watch.go (convertEvent default case)
• Problem: Unrecognized protobuf event types are sent downstream as "UNKNOWN" events. Consumers cannot distinguish intentional unknowns from new server-side enum values this SDK version does not recognize. This can cause subtle bugs in consumer switch statements.
• Fix: Log the unknown type and return nil (already handled as a no-op by the caller), rather than propagating an ambiguous value.

Minor Issues

5. SessionWatchEvent.Type is an untyped string

• File: components/ambient-sdk/go-sdk/types/session_watch.go
• Problem: Raw string literals for event types are less type-safe than typed constants. A type EventType string with named constants would prevent typos and make the API self-documenting. The IsCreated/IsUpdated/IsDeleted helpers mitigate this but do not eliminate it.

6. Closed error channel can yield a nil error in the example

• File: components/ambient-sdk/go-sdk/examples/watch/main.go
• Problem: receiveEvents() closes w.errors on exit. A select consumer reading from the closed channel receives err = nil, logs "Watch error: <nil>", and exits — misleading output. Use a two-value receive (err, ok := <-watcher.Errors()) and check ok before treating the value as an error.

7. tls.Config{} with no minimum version specified

• File: components/ambient-sdk/go-sdk/client/session_watch.go (createGRPCConnection)
• Problem: credentials.NewTLS(&tls.Config{}) relies on Go defaults. Explicitly setting MinVersion: tls.VersionTLS12 documents intent and hardens against future Go default changes.

Positive Highlights

• Clean channel-based streaming API — Events(), Errors(), and Done() channels follow established Go streaming idioms and compose naturally in select statements.
• Correct Stop() implementation — timeoutCancel is cancelled before watchCancel and the connection is closed in the right order. No resource leaks.
• Thorough nil guards in convertSession — Every optional proto field is checked before dereference, preventing panics on partial server data.
• Proper io.EOF handling — receiveEvents() correctly distinguishes normal stream end from error conditions.
• Well-structured example — examples/watch/main.go demonstrates signal handling, context propagation, and the full event loop clearly.

Recommendations

1. (Critical) Remove or implement WatchOptions.ResourceVersion — exporting a field with no effect is a silent API contract violation.
2. (Major) Add unit tests for deriveGRPCAddress, convertEvent, and convertSession before this ships to users.
3. (Major) Return nil from convertEvent for unrecognized event types instead of propagating "UNKNOWN" downstream.
4. (Minor) Introduce a typed EventType constant for the event type field.
5. (Minor) Fix the nil-error channel read in the example.
6. (Minor) Set MinVersion: tls.VersionTLS12 in tls.Config.

---

Review performed by Claude Code using repository standards from .claude/context/ and .claude/patterns/.

[github-actions]

Claude Code Review — PR #814: feat(sdk): add gRPC session watch support

Summary

This PR adds gRPC-based real-time session watching to the Go SDK (SessionWatcher), introduces types/session_watch.go, provides a usage example, and fixes the hardcoded base path across all three SDK flavours. The implementation is mostly well-structured, but two functional bugs block merge and there are a few architectural concerns.

---

BLOCKER: Example code calls NewClientFromEnv with a string — will not compile

components/ambient-sdk/go-sdk/examples/watch/main.go:778

c, err := client.NewClientFromEnv(projectName, client.WithTimeout(120*time.Second))

NewClientFromEnv signature is func NewClientFromEnv(opts ...ClientOption). A string cannot satisfy ClientOption. This fails go build ./... which is explicitly listed in the test plan. The project is now read from AMBIENT_PROJECT; fix by removing the projectName argument:

// export AMBIENT_PROJECT=sdk-demo
c, err := client.NewClientFromEnv(client.WithTimeout(120*time.Second))

---

CRITICAL: WatchOptions.ResourceVersion is declared but silently dropped

components/ambient-sdk/go-sdk/client/session_watch.go:480-528

ResourceVersion is documented as "start watching from (empty = latest)" but WatchSessions is called with an empty request — the field is never used. This is a broken contract. Either pass it to WatchSessionsRequest or remove the field.

---

MAJOR: deriveGRPCAddress uses the HTTP port for gRPC — wrong in dev environments

components/ambient-sdk/go-sdk/client/session_watch.go:726-736

When baseURL is http://localhost:8080, the function returns localhost:8080 for gRPC, not the gRPC port. The 4434 default only activates when the URL has no explicit port. In development, this silently connects to the wrong endpoint. WatchOptions or a ClientOption should allow overriding the gRPC target address.

---

MAJOR: extractBasePath iterates a Go map — non-deterministic result

components/ambient-sdk/generator/parser.go:121-142

Go map iteration order is randomised. If the OpenAPI spec has paths under multiple base prefixes during a migration, the generator could emit different BasePath values across runs. Sort the path keys before iterating, or collect candidates and assert they are all identical.

---

MINOR: Duplicate len(part) > 1 condition

components/ambient-sdk/generator/parser.go:127 — the guard appears twice in the same if; the second occurrence is dead code.

MINOR: Stop() not idempotent — double-close risk

session_watch.go:558-566 — calling Stop() twice calls conn.Close() twice. Guard with sync.Once.

MINOR: Event type should use typed constants

types/session_watch.go:15-28 — comparing Type string against "CREATED" etc. at runtime; exporting a SessionWatchEventType const block would give callers compile-time safety.

MINOR: gRPC connection leaks if context cancelled without Stop()

session_watch.go:568-601 — receiveEvents() exits on ctx.Done() but never closes w.conn; only Stop() does. Consider closing the conn in the goroutine's defer chain.

---

Positive Highlights

• Base path extraction from the OpenAPI spec is a meaningful fix — the hardcoded /api/ambient-api-server/v1 was a maintenance hazard across all three SDK flavours.
• The SessionWatcher channel API (Events(), Errors(), Done()) is idiomatic Go and composes naturally with select.
• convertSession correctly nil-checks every optional pointer field before dereferencing.
• Empty-project validation at NewClient/NewClientFromEnv construction time is a good improvement.
• Removing the ForProject variants simplifies the API surface cleanly.

---

Recommendations

1. (Blocker) Fix NewClientFromEnv call in the example.
2. (Critical) Implement or remove ResourceVersion in WatchOptions.
3. (Major) Make the gRPC address configurable; do not assume it equals the HTTP port.
4. (Major) Sort keys in extractBasePath for deterministic output.
5. (Minor) Items 5-8 can be addressed in a follow-up.

🤖 Generated with Claude Code

[github-actions]

Claude Code Review

Summary: PR 814 adds gRPC session watch to the Go SDK, fixes SDK generator base path derivation, and removes the per-call project override API. The streaming implementation is well-structured. Several issues need attention before merging.

Blocker Issues: None.

Critical Issues

1. Example will not compile - examples/watch/main.go line 21. NewClientFromEnv takes opts ...ClientOption only, but projectName (bare string) is passed as first argument. ClientOption is a function type; a string literal cannot satisfy it. This is a compile-time type error. Project now comes from AMBIENT_PROJECT env per client.go lines 111-113. Remove the first argument. Violates test plan: go build ./... passes in go-sdk/

Major Issues

1. WatchOptions.ResourceVersion silently ignored - session_watch.go line 106. Field documented as version to start watching from but never passed to WatchSessionsRequest. Wire it through or remove the field.

2. Hardcoded ROSA routing in deriveGRPCAddress - session_watch.go lines 277-286. Checking apps.rosa and replacing ambient-api-server embeds cluster topology in the SDK. ARO, HCP, and custom DNS get wrong address silently. Expose WithGRPCAddress ClientOption instead.

3. Compiled binary committed - examples/examples (binary, mode 100755). Add to .gitignore and remove from tree.

Minor Issues

1. parser.go:10 - len(part) > 1 checked twice in same condition.
2. parser.go:19 - SplitN with -1 is identical to Split; prefer strings.Split.
3. parser.go:21 - non-deterministic map iteration can produce inconsistent base paths.

Positive Highlights

• timeoutCancel stored on SessionWatcher and called in Stop() correctly prevents goroutine leak.
• grpc.NewClient replaces deprecated grpc.DialContext+WithBlock correctly.
• url.Parse + net.JoinHostPort used properly in deriveGRPCAddress general case.
• Channel-based API (Events/Errors/Done) is idiomatic Go for streaming.
• url.PathEscape used correctly in the new Delete method (session_api.go:93).
• Empty project validation in NewClient and NewClientFromEnv closes a silent failure mode.
• Removing doForProject/doWithQueryForProject simplifies the client API surface.

Recommendations

1. (Critical) Remove projectName arg from NewClientFromEnv in examples/watch/main.go:21.
2. (Major) Wire opts.ResourceVersion into WatchSessionsRequest or remove the field.
3. (Major) Add WithGRPCAddress option and remove the ROSA heuristic from deriveGRPCAddress.
4. (Major) Remove committed binaries and gitignore them.
5. (Minor) Fix duplicate len check and replace SplitN with Split in parser.go.

[markturansky]

This PR was combined with #804