WIP: Feat/Adds ambient-sdk

[markturansky]

Builds on #637

• adds ambient-sdk
• generates go-sdk, python-sdk, ts-types
• based on the single-source-of-truth in ambient-api-server/openapi/openapi.yaml

[github-actions]

Claude Code Review

Summary

This PR introduces ambient-api-server (a PostgreSQL-backed REST API built on rh-trex-ai framework) and ambient-sdk (Go/Python client libraries with generated types from OpenAPI specs). This is a strategic architecture shift from Kubernetes-native CRDs to a REST API-first approach. The code quality is generally good with 42 test files, comprehensive documentation, and solid security practices. However, there are critical issues that must be fixed before merge.

Overall Assessment: ⚠️ NOT READY FOR MERGE — requires fixes to blocking issues below.

---

Issues by Severity

🚫 Blocker Issues

1. Local Filesystem Dependencies in go.mod

// components/ambient-api-server/go.mod:20
replace github.com/openshift-online/rh-trex-ai => /home/mturansk/projects/src/github.com/openshift-online/rh-trex-ai

• Issue: Hardcoded absolute path to developer's local machine
• Impact: PR won't build on CI/CD or other developer machines
• Files affected:
  • components/ambient-api-server/go.mod
  • Generator templates may have similar issues
• Required fix:
  • Option 1: Use published module version (preferred)
  • Option 2: Use relative replace directive
  • Option 3: Document manual workaround in README with clear instructions

References: CLAUDE.md Git Workflow section - "Branch verification: Always check current branch before file modifications"

---

🔴 Critical Issues

2. Missing RBAC Enforcement in API Handlers

Based on review of the ambient-api-server architecture and comparison with existing backend patterns:

• Issue: Session handlers don't validate user has access to the project before CRUD operations
• Current: X-Ambient-Project header accepted but not validated against user permissions
• Expected: RBAC middleware that checks user's group membership/permissions before allowing operations
• Impact: Security vulnerability - users could access/modify sessions in projects they don't belong to
• Existing pattern (from components/backend/handlers/middleware.go):

  func ValidateProjectContext() gin.HandlerFunc {
      // 1. Extract project from route/header
      // 2. Get user-scoped K8s client from token
      // 3. Check RBAC via SelfSubjectAccessReview
      // 4. Return 403 if unauthorized
  }

Required fix: Add RBAC validation middleware that enforces project isolation

References:

• .claude/context/security-standards.md - "RBAC Enforcement" section
• .claude/patterns/k8s-client-usage.md - "Never bypass namespace checks"
• CLAUDE.md - "Backend and Operator Development Standards > Critical Rules > User Token Authentication Required"

---

3. JSONB Fields Without Schema Validation

Multiple critical fields stored as JSON strings without validation:

// From DATA_MODEL_COMPARISON.md analysis
Repos                *string  // Should be []Repo, stored as JSONB
Conditions           *string  // Should be []Condition
ResourceOverrides    *string  // Should be map/struct
EnvironmentVariables *string  // Should be map[string]string
ReconciledRepos      *string  // Should be []ReconciledRepo

• Issue: No compile-time or runtime validation of JSON structure
• Impact: Malformed JSON can be written to database, causing silent failures when SDK tries to parse
• Risk: Data integrity issues, difficult debugging

Required fix:

• Add proper Go structs for these types
• Add validation in handlers before writing to DB
• Document expected schemas in OpenAPI specs

References: CLAUDE.md - "Type-Safe Unstructured Access" pattern

---

🟡 Major Issues

4. Incomplete Session Model vs Backend

Major gaps identified in DATA_MODEL_COMPARISON.md:

Missing Fields (from existing backend CRD):

• interactive (bool) - Required for long-running chat sessions
• timeout (int) - Session timeout configuration
• llm_model, llm_temperature, llm_max_tokens - LLM configuration
• parent_session_id - Session cloning/forking
• Status fields: phase, start_time, completion_time, conditions, reconciled_repos

Missing Endpoints:

• POST /sessions/{id}/start - Trigger session start
• POST /sessions/{id}/stop - Trigger session stop
• POST /sessions/{id}/clone - Clone to another project
• File workspace operations (GET/PUT/DELETE /sessions/{id}/workspace/*)
• Git operations (GET /sessions/{id}/git/status, etc.)

Impact:

• SDK cannot replicate full backend functionality
• Critical for WaitForCompletion() pattern in SDKs
• Breaks migration path from K8s-native backend

Required: Add missing fields and endpoints before this can replace the backend

References:

• components/ambient-api-server/DATA_MODEL_COMPARISON.md lines 28-78
• Existing backend: components/backend/handlers/sessions.go

---

5. Multi-Repo Support Incomplete

• Issue: Backend supports spec.repos[] array, API server has single repo_url field
• Current workaround: Repos *string field exists but stored as unparsed JSON
• Impact: Cannot migrate sessions with multiple repositories
• Required: Proper many-to-many relationship or structured array type

References: DATA_MODEL_COMPARISON.md line 32

---

6. Missing Control-Plane Reconciler

• Issue: The architecture requires a control-plane component to sync Postgres → Kubernetes CRs
• Status: Mentioned in docs but not implemented in this PR
• Impact: API server writes to Postgres but operator reads K8s CRs - no bridge
• Data flow broken:

  Frontend → ambient-api-server → Postgres
                                      ↓ (MISSING reconciler)
                                   K8s CRs ← operator watches
  

Required: Either include control-plane in this PR or clearly document it as follow-up work

References:

• DATA_MODEL_COMPARISON.md lines 12-16
• components/ambient-api-server/CLAUDE.md

---

🔵 Minor Issues

7. Inconsistent JSON Field Naming

• Database: snake_case (PostgreSQL conventions)
• OpenAPI: mix of camelCase and snake_case
• SDK types: inconsistent (Go SDK has both AssignedUserID and assigned_user_id)

Recommendation: Document naming convention strategy in ambient-api-server/CLAUDE.md

---

8. No CI/CD Integration

• New components not added to .github/workflows/components-build-deploy.yml
• No build/test jobs for ambient-api-server or ambient-sdk
• Impact: Changes won't be validated in CI

Recommendation: Add GitHub Actions workflows before merge

---

9. Missing Integration Tests

• Go SDK has client_test.go but limited coverage
• Python SDK has test script but not integrated into CI
• No end-to-end tests between SDK and API server

Recommendation: Add smoke tests that validate SDK → API server communication

---

Positive Highlights

1. Excellent Documentation ✅

   • Comprehensive CLAUDE.md files for each component
   • DATA_MODEL_COMPARISON.md provides clear migration path
   • Plugin anatomy guide for future Kind development

2. Security-Conscious Design ✅

   • SecureToken type with automatic log redaction (slog.LogValuer)
   • Token validation at SDK client construction
   • Bearer token authentication with proper header handling
   • Production environment enforces HTTPS and auth

3. Type Safety ✅

   • OpenAPI-first approach ensures SDK stays in sync
   • Generated types for Go/Python SDKs
   • Strong typing throughout (minimal interface{} usage)

4. Modularity ✅

   • Clean plugin architecture (12 Kinds, each self-contained)
   • Generator templates for extending with new Kinds
   • Separation: API layer → Service → DAO → Database

5. Test Coverage ✅

   • 42 test files included across plugins
   • Unit tests, integration tests, migration tests
   • Framework provides test harness (testcontainers for Postgres)

6. OpenAPI Specifications ✅

   • Modular YAML files (one per Kind)
   • Well-structured schemas
   • Canonical contract for SDK generation

7. Framework Leverage ✅

   • rh-trex-ai eliminates boilerplate
   • Battle-tested patterns from OpenShift Online
   • Built-in features: migrations, Tree Search Language, OpenTelemetry

---

Recommendations

Before Merge (Priority Order):

1. Fix go.mod replace directives (blocker)
2. Add RBAC middleware to enforce project isolation (security critical)
3. Add schema validation for JSONB fields (data integrity)
4. Document missing fields/endpoints as known gaps with timeline
5. Add CI/CD workflows for new components
6. Fix SDK test coverage and add smoke tests

Follow-Up Work (Document in Issues):

7. Implement control-plane reconciler (Postgres → K8s sync)
8. Add missing Session endpoints (start/stop/clone/workspace/git)
9. Implement AG-UI protocol (SSE streaming for events)
10. Add multi-repo support (proper array handling)
11. Migrate frontend to use SDK instead of direct backend calls
12. Add TypeScript SDK generation

---

Code Quality Checklist Status

Based on CLAUDE.md standards:

Backend/API Server:

• ❌ Authentication: Missing user token validation in handlers
• ⚠️ Authorization: RBAC not enforced (critical gap)
• ✅ Error Handling: No panics, proper error returns
• ✅ Token Security: Redaction implemented via SecureToken type
• ⚠️ Type Safety: JSONB fields bypass type checking
• ✅ Resource Cleanup: Not applicable (no K8s resources created)
• ✅ Logging: Structured logs with context
• ❌ Code Quality: go.mod replace directive prevents builds

SDK:

• ✅ Type Safety: Generated types from OpenAPI
• ✅ Error Handling: Structured error types, no panics
• ✅ Token Security: Validation on construction, log redaction
• ⚠️ Testing: Limited integration test coverage

---

Final Verdict

This PR represents excellent foundational work for the Postgres-backed REST API architecture. The code quality, documentation, and design patterns are solid. However, the PR cannot be merged until:

1. ✅ Local filesystem dependencies are resolved
2. ✅ RBAC enforcement is added to handlers
3. ✅ JSONB schema validation is implemented

Once these three blockers are addressed, this PR will provide a strong foundation for migrating from the Kubernetes-native backend to a REST API-first approach.

Estimated effort to fix blockers: 4-8 hours
Recommended approach: Fix #1 immediately, then address #2 and #3 in parallel

---

Review performed by Claude Code using repository standards from CLAUDE.md, .claude/context/, and .claude/patterns/
Standards applied: Backend Development Context, Security Standards, K8s Client Usage Patterns, Error Handling Patterns

---

🔍 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:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. 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.