feat: Auto-generate and edit session display names

[Gkrumbach07]

Summary

Two-part feature for session display name management:

1. Auto-Generate Display Names

When users send their first message in a session, the backend automatically generates a descriptive display name using Claude Haiku.

• Triggers on first user message when displayName is empty
• Uses session context (repos, workflow) to generate relevant names
• Runs asynchronously - fails silently with logging on error
• Supports both Anthropic API (via project secret) and Google Vertex AI

2. Manual Name Editing

Added UI controls to edit session names:

• "Edit name" option in session detail page kebab menu
• "Edit name" option in sessions list actions dropdown
• Simple modal dialog for editing

Backend Changes

• Added handlers/display_name.go with Claude Haiku integration
• Added PUT /api/projects/:projectName/agentic-sessions/:sessionName/displayname endpoint
• Modified PostSessionMessageWS to trigger auto-generation
• Added Anthropic Go SDK with Vertex AI support

Frontend Changes

• Added EditSessionNameDialog component
• Added useUpdateSessionDisplayName React Query mutation
• Added Next.js API proxy route for display name updates
• Updated session-header.tsx and sessions-section.tsx with edit buttons

Testing

• Tested manual name editing via UI
• Verified backend deployment with Vertex AI authentication fix

[github-actions]

Claude Code Review

Summary

This PR adds auto-generated and manual display name editing for sessions. The implementation is mostly solid with good patterns, but contains several critical security and architecture issues that must be addressed before merge.

Issues by Severity

🚫 Blocker Issues

1. Missing RBAC Permission Check in PUT Endpoint (handlers/sessions.go:1004-1086)

• Issue: UpdateSessionDisplayName checks user auth but NEVER performs RBAC check
• Risk: Users can modify sessions they shouldn't have write access to
• Pattern Violation: CLAUDE.md Pattern 3 (Authorization Errors) - always check RBAC before operations
• Fix Required:

// After line 1013, add:
ssar := &authv1.SelfSubjectAccessReview{
    Spec: authv1.SelfSubjectAccessReviewSpec{
        ResourceAttributes: &authv1.ResourceAttributes{
            Group:     "vteam.ambient-code",
            Resource:  "agenticsessions",
            Verb:      "update",
            Namespace: project,
        },
    },
}
res, err := reqK8s.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, ssar, v1.CreateOptions{})
if err \!= nil || \!res.Status.Allowed {
    c.JSON(http.StatusForbidden, gin.H{"error": "Unauthorized to update session"})
    return
}

2. Goroutine Leak Risk (display_name.go:76-82)

• Issue: GenerateDisplayNameAsync launches goroutine without lifecycle management
• Risk: If session is deleted during generation, goroutine continues unnecessarily
• Current Mitigation: updateSessionDisplayNameInternal checks IsNotFound (line 273-276) - this is GOOD
• Concern: 10s timeout is appropriate, but no way to cancel if backend restarts
• Recommendation: Document that goroutines are bounded by displayNameAPITimeout (10s max)

🔴 Critical Issues

3. Potential API Key Exposure in Logs (display_name.go:160)

• Issue: Log message at line 160 logs "Using API key for {project}"
• Risk: While not logging the key itself, confirms which projects have API keys configured
• Pattern Violation: Security-standards.md - minimize information disclosure
• Fix: Change log level to Debug or remove entirely

4. Missing Error Context in Async Generation (display_name.go:78-79)

• Issue: Errors are logged but not tracked in session status
• UX Impact: User never knows if display name generation failed
• Recommendation: Consider updating session annotations with generation status
• Example: Add annotation ambient-code.io/displayname-generation: "failed"

5. Type Safety Violation in SessionContext (display_name.go:331-350)

• Issue: ExtractSessionContext uses direct type assertions without checking
• Pattern Violation: CLAUDE.md Rule 4 (Type-Safe Unstructured Access)
• Risk: Will panic if spec structure changes
• Fix Required (line 333):

// Current (line 333):
if repos, ok := spec["repos"].([]interface{}); ok {

// Better:
repos, found, err := unstructured.NestedSlice(spec, "repos")
if err \!= nil || \!found {
    // Handle error
}

🟡 Major Issues

6. Inconsistent Client Usage Pattern (display_name.go:166-168, 264)

• Issue: Uses K8sClientProjects instead of standard K8sClient
• Confusion: Backend has both K8sClient and K8sClientProjects - unclear why
• Question: Is K8sClientProjects the backend service account client?
• Recommendation: Document the distinction or consolidate to K8sClient

7. No Rate Limiting on Display Name Generation (display_name.go:76)

• Issue: User could spam first messages to trigger many Claude API calls
• Risk: Cost/quota issues if abused
• Mitigation: Generation only triggers when displayName is empty (line 292-295) - GOOD
• Recommendation: Add rate limiting or debouncing if this becomes an issue

8. WebSocket Integration Not Using User Token (websocket/handlers.go:86-124)

• Issue: Cannot determine if auto-generation code path gets user context
• Concern: Need to verify that PostSessionMessageWS has access to projectName/sessionName
• Question: Where is PostSessionMessageWS that calls GenerateDisplayNameAsync?
• Recommendation: Ensure session context is properly extracted from WebSocket message

🔵 Minor Issues

9. Frontend: Missing Error Toast on Mutation Failure

• Issue: useUpdateSessionDisplayName invalidates queries on success but no user feedback on error
• UX: User won't know if save failed
• Fix: Add onError callback with toast notification

10. Frontend: Dialog Component Uses Props Correctly ✅

• Good: Resets input on open (line 35-39)
• Good: Validates input before enabling save button
• Good: Shows loading state with spinner
• Good: Uses Shadcn UI components exclusively

11. Validation Logic Duplication

• Issue: Validation in frontend (maxLength={50}) and backend (maxDisplayNameLength)
• Risk: If constants get out of sync, confusing UX
• Recommendation: Consider exposing backend constants via API endpoint

12. Log Injection Sanitization ✅

• Good: sanitizeDisplayName properly removes control characters (line 47-54)
• Good: Uses regex to prevent log injection
• Pattern Adherence: Follows security-standards.md validation patterns

Positive Highlights

✅ Type-Safe Unstructured Access (display_name.go:282-288, handlers/sessions.go:1044-1052)

• Properly uses unstructured.NestedMap with three-value returns
• Checks found before using values
• Excellent adherence to CLAUDE.md Rule 4

✅ User Token Authentication (handlers/sessions.go:1007-1013)

• PUT endpoint correctly uses GetK8sClientsForRequest
• Returns 401 if token invalid
• Follows authentication pattern from backend-development.md

✅ Proper Error Handling (display_name.go:273-276, 307-310)

• IsNotFound errors treated as non-fatal (correct for async operations)
• Goroutine exits gracefully if session deleted
• Follows error-handling.md patterns

✅ Input Validation (display_name.go:59-71, handlers/sessions.go:1024-1027)

• Validates display name length, emptiness, control characters
• Uses Unicode-aware truncation (utf8.RuneCountInString)
• Sanitizes user input to prevent injection

✅ React Query Best Practices (use-sessions.ts:349-375)

• Mutation properly structured with mutationFn
• Invalidates both detail and list queries on success
• Follows react-query-usage.md patterns

✅ Shadcn UI Components (edit-session-name-dialog.tsx)

• Zero custom UI components
• Proper use of Dialog, Button, Input, Label
• Follows frontend-development.md standards

✅ Context Timeout (display_name.go:86)

• 10s timeout prevents hanging goroutines
• Appropriate for external API call

Recommendations

Prioritized Action Items

Must Fix Before Merge:

1. Add RBAC permission check in UpdateSessionDisplayName (Issue Outcome: Reduce Refinement Time with agent System #1)
2. Fix type assertions in ExtractSessionContext to use unstructured helpers (Issue Epic: Jira Integration & Workflow #5)

Should Fix Before Merge:
3. Remove or reduce log verbosity for API key usage (Issue #3)
4. Document K8sClientProjects vs K8sClient distinction (Issue #6)
5. Add error toast notification in frontend mutation (Issue #9)

Nice to Have:
6. Add session annotation for display name generation status (Issue #4)
7. Verify WebSocket integration properly extracts session context (Issue #8)

Architecture Questions

1. Where is PostSessionMessageWS? The PR description mentions it but I don't see changes to it in the diff
2. Why K8sClientProjects instead of K8sClient? Is this a different service account?
3. Should display name generation status be visible to users? Currently fails silently

Testing Recommendations

• Test PUT endpoint with user lacking update permission (should get 403)
• Test auto-generation with session deleted mid-generation (should not error)
• Test display name with Unicode characters (emoji, CJK, etc.)
• Test with >50 character input (should truncate properly)
• Test rapid session creation to verify no goroutine leaks
• Test Vertex AI vs API key paths (both code paths should work)

Conclusion

This is well-structured code that follows most project patterns. The display name generation feature is thoughtfully designed with proper async handling and input validation. However, the missing RBAC check is a security vulnerability that blocks merge. Fix issues #1 and #5, address the recommendations, and this will be ready to ship.

Great work on:

• Type-safe unstructured access
• Proper error handling in async operations
• Security-conscious input sanitization
• Following React Query and Shadcn UI patterns

🔒 Security Score: 7/10 (would be 9/10 with RBAC fix)
🏗️ Architecture Score: 8/10 (solid patterns, minor inconsistencies)
✅ Code Quality Score: 9/10 (excellent adherence to guidelines)

---

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

[github-actions]

Claude Code Review

Summary

PR #432 adds display name management for agentic sessions. The implementation demonstrates strong adherence to project standards after addressing code review feedback.

Issues by Severity

Blocker Issues

None. All blockers addressed in commits dfe6c3c and b989f5f.

Critical Issues

1. Missing RBAC Check in Internal Update Function (display_name.go:268)

• Issue: updateSessionDisplayNameInternal uses backend service account without clear indication
• Recommendation: Rename to updateSessionDisplayNameAsBackend with comment

2. Potential Token Leakage in Error Paths (display_name.go:157-159)

• Issue: Error wrapping might expose secret names in logs
• Recommendation: Sanitize error message

Major Issues

3. Missing Structured Error Responses (Next.js API route)

• Issue: Generic error messages in displayname/route.ts:24
• Recommendation: Return specific error codes (403, 404, etc)

4. Goroutine Documentation (display_name.go:82)

• Recommendation: Add metric/log for pending goroutines count

Minor Issues

• Hardcoded secret name (extract to constants)
• Log verbosity (use debug level)
• Input validation timing (add character whitelist)
• Race condition documentation

Positive Highlights

• Excellent security: RBAC checks, user token validation, token redaction
• Type safety: Consistent use of unstructured helpers
• Error handling: Follows CLAUDE.md patterns
• Frontend compliance: Shadcn UI, React Query, loading states
• Code organization: Isolated logic, clear separation of concerns

Recommendations

High Priority (Before Merge)

1. Rename internal function to updateSessionDisplayNameAsBackend
2. Sanitize API key error messages
3. Improve Next.js error handling

Medium Priority (Post-Merge)

4. Add character whitelist validation
5. Extract hardcoded secret name
6. Reduce log verbosity
7. Add goroutine metrics

Architecture Compliance

✅ Backend: User token auth, RBAC, type-safe access, no panics
✅ Frontend: Zero any types, Shadcn UI, React Query, proper types

Conclusion

Strong engineering practices and diligent attention to code review feedback. All blocker issues addressed. Remaining issues are documentation clarity and minor enhancements.

Recommendation: APPROVE with minor post-merge improvements

---

Review completed by Claude Code using project standards

---

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

[github-actions]

Claude Code Review

Summary

This PR adds auto-generated session display names and manual editing capabilities. The feature triggers Claude Haiku to generate descriptive names on first user message and provides UI controls for manual editing. Implementation spans backend API, WebSocket handlers, and frontend UI components.

Overall Assessment: Well-structured feature with good separation of concerns. Several critical security and architecture issues require immediate attention before merge.

---

Issues by Severity

🚫 Blocker Issues

1. SECURITY: Missing User Token Authentication in Display Name Update (components/backend/handlers/sessions.go:1118-1148)

• ✅ FIXED in latest commit: RBAC check now properly uses user token (reqK8s)
• Properly validates user has update permission on agenticsessions resource
• Good defensive programming - returns 401/403 with appropriate error messages
• Status: RESOLVED ✓

2. CRITICAL: Goroutine Leak Risk (components/backend/handlers/display_name.go:82-88)

• GenerateDisplayNameAsync spawns unbounded goroutines with no tracking
• Pod restarts leave orphaned goroutines (mitigated by 10s timeout but not ideal)
• No way to cancel in-flight requests when session is deleted
• Recommendation: Consider using a worker pool pattern or at minimum document the goroutine lifecycle
• Current timeout (10s) provides some protection but better would be context cancellation tied to request lifecycle

🔴 Critical Issues

3. Inconsistent K8s Client Usage (components/backend/handlers/display_name.go:172-187)

• getAPIKeyFromSecret uses K8sClientProjects (service account) directly
• This is acceptable here since we're reading project secrets (not user operation)
• However, variable name K8sClientProjects is confusing - should document this is the service account client
• Recommendation: Add comment explaining why service account is appropriate here

4. Missing Input Validation on HTTP Handler (components/backend/handlers/sessions.go:1156-1160)

• UpdateSessionDisplayName validates binding but doesn't call ValidateDisplayName
• Allows invalid names to reach CR update (control characters, length violations)
• Required Fix:

var req struct {
    DisplayName string `json:"displayName" binding:"required"`
}
if err := c.ShouldBindJSON(&req); err != nil {
    c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request"})
    return
}

// ADD THIS VALIDATION
if errMsg := ValidateDisplayName(req.DisplayName); errMsg != "" {
    c.JSON(http.StatusBadRequest, gin.H{"error": errMsg})
    return
}

5. Race Condition in First Message Detection (components/backend/websocket/handlers.go:218-275)

• Checks message count before message is persisted (userMessageCount > 1)
• Comment acknowledges this: "Since we're checking before persist fully completes"
• Could trigger name generation multiple times if messages arrive rapidly
• Impact: Low (redundant API calls, race condition in updateSessionDisplayNameInternal:296-300 provides mitigation)
• Recommendation: Move check after message persistence or use atomic counter

🟡 Major Issues

6. Error Handling: Silent Failures in Goroutine (components/backend/handlers/display_name.go:82-88)

• Only logs errors, no way to surface failures to user
• User has no indication if name generation failed (e.g., API key invalid, quota exceeded)
• Recommendation: Consider updating session status/annotations to reflect generation state
• Alternative: Emit K8s Event for debugging

7. Type Safety: Direct Type Assertions Without Checking (components/backend/websocket/handlers.go:252)

• spec, ok := session["spec"].(map[string]interface{}) - good ✓
• But returns silently on failure - should log for debugging
• Recommendation: Add log statement when type assertion fails

8. API Client Initialization: Context from Background (components/backend/handlers/display_name.go:92)

• Creates new context.Background() instead of propagating request context
• Loses trace IDs, cancellation signals from original request
• Impact: Can't cancel if initiating request is aborted
• Fix: Pass context through from websocket handler

9. Frontend: Missing Error States in Dialog (components/frontend/src/components/edit-session-name-dialog.tsx)

• Dialog shows loading state but no error state
• If mutation fails, user gets no visual feedback (relies on toast)
• Recommendation: Add error message display in dialog

🔵 Minor Issues

10. Code Organization: Large Display Name File (components/backend/handlers/display_name.go:362 lines)

• Good separation of concerns ✓
• Consider splitting into:
  • display_name.go - HTTP handler + validation
  • display_name_gen.go - Claude API integration
  • display_name_client.go - Anthropic client initialization

11. Logging: Inconsistent Prefixes

• Uses DisplayNameGen: prefix inconsistently
• Example: Line 149 has it, line 167 doesn't
• Recommendation: Use structured logging with fields instead of prefixes

12. Hardcoded Values: Secret Name (components/backend/handlers/display_name.go:23)

• runnerSecretsName = "ambient-runner-secrets" is hardcoded
• Should be configurable or at minimum documented why it's hardcoded
• Impact: Low (matches established pattern elsewhere)

---

Positive Highlights

✅ Excellent Type Safety: Consistent use of unstructured.Nested* helpers (display_name.go:288-293, 297, 340-361)

✅ Security Best Practices:

• Token redaction in logs (no API keys logged)
• Control character sanitization (display_name.go:44-54)
• Unicode-aware length validation using utf8.RuneCountInString

✅ Good Error Handling Patterns:

• Checks errors.IsNotFound appropriately (display_name.go:279-283, 313-316)
• Graceful degradation when session deleted during generation

✅ Frontend Excellence:

• Zero any types ✓
• Uses Shadcn UI components (Dialog, Button, Input) ✓
• React Query integration for mutations ✓
• Proper loading states on buttons

✅ Documentation: Goroutine lifecycle clearly documented (display_name.go:77-81)

✅ Vertex AI Support: Thoughtful implementation of both Anthropic API and Vertex AI auth paths

---

Recommendations

Priority 1 (Before Merge)

1. Add input validation in UpdateSessionDisplayName handler (Issue Epic: AI Agent Development #4)
2. Add error state to EditSessionNameDialog component (Issue Bump actions/add-to-project from 0.5.0 to 1.0.2 #9)
3. Document goroutine lifecycle risks or implement worker pool (Issue Epic: RAT Architecture & Design #2)

Priority 2 (Follow-up PR)

4. Fix race condition in first message detection (Issue Epic: Jira Integration & Workflow #5)
5. Propagate request context through async generation (Issue Test: Updated Workflow Validation #8)
6. Add structured logging (Issue Bump actions/github-script from 6 to 7 #11)

Priority 3 (Nice-to-Have)

7. Split display_name.go into smaller files (Issue Bump actions/checkout from 4 to 5 #10)
8. Add session status/events for generation failures (Issue Epic: Testing & Validation #6)

---

Testing Recommendations

• Test with rapid message submission (race condition check)
• Test with invalid API key (error handling path)
• Test with session deletion during name generation (goroutine cleanup)
• Test with extremely long user messages (prompt size limits)
• Test RBAC enforcement (user without update permission)
• Test with both Anthropic API and Vertex AI configurations

---

Commit Before Merge

Please address Issues #4 and #9 before merging. The RBAC security issue (#1) has been properly resolved in the latest commit. Great work on implementing user token authentication!

---

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