Skip to content

WIP: Feat/Frontend to consume new v2 API#640

Open
markturansky wants to merge 3 commits intoambient-code:mainfrom
markturansky:feat/frontend_to_api
Open

WIP: Feat/Frontend to consume new v2 API#640
markturansky wants to merge 3 commits intoambient-code:mainfrom
markturansky:feat/frontend_to_api

Conversation

@markturansky
Copy link
Contributor

@markturansky markturansky commented Feb 16, 2026

Jira: RHOAIENG-51896

Builds on #639

  • Adds v2 API parallel paths to UI.
  • Keeps original Kube APIs for testing purposes

@github-actions

This comment has been minimized.

@markturansky markturansky changed the title Feat: Frontend to consume new v2 API WIP: Feat/Frontend to consume new v2 API Feb 16, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 28, 2026

Claude Code Review

Summary

This PR adds a dual-API architecture to the frontend, allowing users to toggle between the existing Kubernetes-backed API and a new v2 API server. It introduces an ApiSourceContext, v1 React Query hooks wrapping the TypeScript SDK, workspace section components, and an adapter layer normalizing v1 responses into AgenticSession shape. Vitest testing infrastructure is also added.

The architecture is sound and the dual-hook pattern with enabled flags is idiomatic React Query. Several issues should be addressed before merging.

Issues by Severity

Blocker Issues

1. Hardcoded Bearer no-auth token in useDeleteSessionDual

src/services/queries/use-session-dual.ts lines 63-70 issue a raw fetch with a hardcoded fake credential in the Authorization header. This is the only v1 mutation that bypasses createAmbientClient — all others (useStopSessionDual, useContinueSessionDual) correctly delegate to it. This must be replaced with createAmbientClient(projectName).sessions.delete(sessionName).

Critical Issues

2. Token resolution silently falls through to 'no-auth' in browser contexts

src/lib/ambient-client.ts reads process.env.OC_TOKEN, which is a server-side env var. Next.js does not expose non-NEXT_PUBLIC_ vars in browser bundles. Since most v1 hooks call createAmbientClient(project) without an explicit token, browser requests silently fall back to 'no-auth'. The K8s path correctly derives the user token from auth context; the v1 path needs the same treatment.

3. ApiSourceToggle unconditionally rendered in production navigation

src/components/navigation.tsx renders <ApiSourceToggle /> with no feature flag or environment gate. This exposes a testing/developer toggle to all production users. Gate it behind FeatureFlagProvider (already in the project) or an isDevelopment check.

Major Issues

4. npm install instead of npm ci in Dockerfile — non-reproducible builds

The Dockerfile mutates package.json with sed at build time then runs npm install. This is fragile: npm install does not guarantee lock-file fidelity. Consider vendoring the SDK at a stable path that matches package.json without modification, and restore npm ci.

5. Component size violations

Several new components exceed the 200-line guideline from COMPONENT_PATTERNS.md:

  • v1-project-keys-section.tsx — 427 lines
  • v1-permissions-section.tsx — 287 lines
  • v1-sessions-section.tsx — 237 lines
  • v1-create-session-dialog.tsx — 226 lines

Extract table rows, action dialogs, and pagination controls as dedicated sub-components.

6. createAmbientClient instantiated on every render

Every queryFn / mutationFn calls createAmbientClient(project) inline, constructing a new SDK client per render cycle. Consider a module-level cache keyed by (baseUrl, project, token) or useMemo-based memoisation.

Minor Issues

7. Default API source is 'api-server', not 'k8s'

getInitialSource() returns 'api-server' when no preference is stored, meaning new users land on the experimental v1 API. Defaulting to 'k8s' is safer until v1 reaches production parity.

8. v1 sessionKeys.detail omits project — potential cache collision

The v1 key factory uses detail: (id: string) => [...] while the K8s factory includes projectName. Sessions in different projects with the same ID will share a React Query cache entry.

9. Stale time comments stripped from existing hooks

Adding the enabled option removed inline documentation (e.g., // 5 minutes - user info doesn't change often in use-auth.ts). These comments should be preserved.

10. Turbopack root conditional in next.config.js

The turbopack.root config is now dev-only. Verify this doesn't silently affect the production build (the original comment cited a monorepo warning suppression that may apply in all environments).

Positive Highlights

  • Clean adapter patternv1-session-adapter.ts with the generic parseJsonField<T> is type-safe and well-guarded against malformed JSON.
  • Dual-hook patternuseSession(…, { enabled: !isApiServer }) is idiomatic React Query and correctly avoids unnecessary requests.
  • Vitest setup — Well-structured with jsdom, path aliases, and @testing-library/react. Tests for api-source-context and v1/keys provide meaningful coverage.
  • Query key factoryv1/keys.ts follows the established project pattern; tests verify uniqueness across all 13 resources.
  • Consistent Shadcn UI — All new components use @/components/ui/* correctly.
  • Zero any types — The PR maintains the zero-any requirement throughout.
  • useApiSourceOptional — Good defensive API design for use outside the provider.

Recommendations

  1. Fix useDeleteSessionDual — Replace inline fetch + hardcoded 'Bearer no-auth' with createAmbientClient(projectName).sessions.delete(sessionName).
  2. Fix token resolution — Pass the authenticated user token into createAmbientClient from auth context.
  3. Gate ApiSourceToggle — Feature flag or isDevelopment check before merging to production.
  4. Fix Dockerfile — Stable SDK vendor path, restore npm ci.
  5. Split large components — Components over 200 lines need decomposition.
  6. Default to 'k8s' — Safer default until v1 API is production-ready.
  7. Add project to v1 session detail keydetail: (project: string, id: string) => [...].

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

markturansky and others added 3 commits March 3, 2026 11:25
… SDK integration

Introduces the ambient-control-plane component — a Go controller that watches
the ambient-api-server via gRPC streams and reconciles desired state into
Kubernetes CRs (AgenticSession, Project, ProjectSettings).

Control plane features:
- gRPC watch + paginated list-sync informer for real-time event delivery
- Session, Project, and ProjectSettings reconcilers for K8s CR management
- Optimistic concurrency retry on 409 conflict during CR updates
- Write-back echo detection to prevent infinite update loops
- Local mode with runner process management and AG-UI reverse proxy
- Configurable namespace targeting via NAMESPACE env var

SDK changes (required dependency):
- URL path updated to /api/ambient/v1
- NewClientFromEnv requires explicit project parameter
- gRPC session watch client for real-time stream subscriptions
- Watch example added

Also adds CI workflow for control plane unit tests.
- Added ApiSourceToggle to navigation component
- Wrapped app layout with ApiSourceProvider
- Enables switching between Kubernetes and API Server sources

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@ambient-code
Copy link

ambient-code bot commented Mar 4, 2026

Merge Readiness — Blockers Found

Check Status Detail
CI FAIL Failing: Unit Tests, test-local-dev-simulation, CLI Build and Tests (+1 more)
Merge conflicts pass
Review comments FAIL Has comments — agent to evaluate
Jira hygiene pass
Staleness pass
Diff overlap risk FAIL Line overlap with #747, #778, #794, #801 on components/ambient-sdk/go-sdk/client/client.go, components/ambient-sdk/go-sdk/client/iterator.go

This comment is auto-generated by the PR Overview workflow and will be updated when the PR changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant