Skip to content

Comments

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

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

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

Conversation

@markturansky
Copy link

Builds on #639

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2026

Claude Code Review

Summary

PR #640 successfully adds parallel V2 API support to the frontend alongside existing Kubernetes API paths, implementing proper React Query integration, context management, and type adapters. However, there are 3 critical security/authentication issues that block production readiness.

Issues by Severity

Blocker Issues

1. CRITICAL: Hardcoded 'no-auth' Token Fallback

  • Files: components/frontend/src/lib/ambient-client.ts:12, components/frontend/src/services/queries/use-session-dual.ts:69
  • Issue: The ambient client falls back to hardcoding 'no-auth' as a bearer token when no authentication source is available
  • Why blocker: Violates security standards, works in dev mode but fails mysteriously in production, creates false confidence
  • Required fix: Explicitly error when no valid token is available. Never use hardcoded fallback tokens.

2. Auth Configuration Mismatch: Dev vs. Production

  • Files: components/ambient-api-server/cmd/ambient-api-server/environments/e_development.go:21
  • Issue: Development environment explicitly disables JWT but production uses enable-authz: true
  • Why blocker: Testing appears to work with 'no-auth' in dev, masking production authentication failures
  • Required fix: Add authentication environment detection or always require valid tokens (even in dev).

3. Missing Project Context Validation

  • File: components/frontend/src/services/queries/use-session-dual.ts:70
  • Issue: Delete mutation silently falls back to 'default' project: X-Ambient-Project: projectName || 'default'
  • Why blocker: Silent cross-project operations if projectName is undefined; potential data loss
  • Required fix: Throw error if projectName is missing. Never default to a different project context.

Critical Issues

4. Inconsistent Error Handling in Delete Mutation

  • File: components/frontend/src/services/queries/use-session-dual.ts:73-74
  • Issue: Confusing error condition logic that treats 204 specially
  • Impact: May not properly detect HTTP errors

5. Type Safety Violation in Adapter

  • File: components/frontend/src/lib/v1-session-adapter.ts:51
  • Issue: Unsafe type assertion without validation
  • Why critical: Violates Zero any Types rule from DESIGN_GUIDELINES.md

6. No Token Validation in SDK Client

  • File: components/ambient-sdk/ts-sdk/src/base.ts:77-124
  • Issue: ambientFetch doesn't validate tokens before making requests
  • Impact: No client-side feedback for authentication issues

7. Missing Error Handling in Mutations

  • File: components/frontend/src/services/queries/use-session-dual.ts:45-56, 91-99
  • Issue: V1 mutations don't have error toast/notification handlers
  • Fix: Add onError handlers with toast notifications

Major Issues

8. Test Validates Insecure Behavior - components/frontend/src/lib/ambient-client.test.ts:9
9. No Graceful Degradation Between API Sources - components/frontend/src/app/projects/[name]/page.tsx:55-69
10. Missing Migration Documentation - No documentation on when/how to migrate from K8s API to V2 API

Minor Issues

11. Type Definition Duplication - components/frontend/src/lib/v1-session-adapter.ts:1-9
12. Refetch Interval Logic Hardcoded - components/frontend/src/services/queries/v1/use-sessions.ts:25-32
13. Query Invalidation Pattern Incomplete - components/frontend/src/services/queries/use-session-dual.ts:50-51
14. No SDK Regeneration Process Documented - components/ambient-sdk/ts-sdk/src/index.ts:1

Positive Highlights

✅ Clean architectural separation: SDK → adapter → React Query hooks
✅ Proper React Query patterns: Uses useQuery/useMutation consistently
✅ Type safety: Strong typing throughout (except noted violations)
✅ Backward compatibility: K8s APIs retained for testing
✅ Context management: ApiSourceContext allows runtime switching
✅ Comprehensive OpenAPI specs: Well-documented API surface

Recommendations

Immediate (Before Merge)

  1. Replace 'no-auth' hardcoded fallback with explicit error throwing
  2. Add project context validation in all V2 API mutations
  3. Fix error handling in delete mutation condition
  4. Add runtime type validation in adapter functions
  5. Add error toast handlers to all mutations

High Priority (Next PR)

  1. Document authentication flow for V2 API in CLAUDE.md
  2. Add token validation to SDK client constructor
  3. Implement graceful fallback between K8s and V2 APIs
  4. Document migration strategy and deprecation timeline

Conclusion

This PR represents solid engineering work with good architectural decisions, but cannot be merged in its current state due to authentication security issues. The hardcoded 'no-auth' fallback (Blocker #1) must be fixed before deployment to any environment.

Estimated effort to address blockers: 2-4 hours
Recommendation: Request changes, provide revised authentication implementation, then re-review.


Review performed by Claude Code using repository standards from .claude/context/ and .claude/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.

@markturansky markturansky changed the title Feat: Frontend to consume new v2 API WIP: Feat/Frontend to consume new v2 API Feb 16, 2026
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