feat: add capabilities endpoint and enhance AGUI event handling#613
feat: add capabilities endpoint and enhance AGUI event handling#613Gkrumbach07 wants to merge 2 commits intoambient-code:mainfrom
Conversation
- Introduced a new endpoint for retrieving runner capabilities at `/agentic-sessions/:sessionName/agui/capabilities`. - Implemented the `HandleCapabilities` function to authenticate users, verify permissions, and proxy requests to the runner. - Enhanced AGUI event handling by adding support for custom events and persisting message snapshots for faster reconnections. - Updated the frontend to utilize the new capabilities endpoint and replaced the existing chat component with `CopilotChatPanel` for improved user experience. This update improves the overall functionality and performance of the AG-UI system, allowing for better integration with the runner's capabilities and enhancing user interactions.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment has been minimized.
This comment has been minimized.
- Fixed a typo in the event type constant from `EventTypStateDelta` to `EventTypeStateDelta`. - Added a new event type constant `EventTypeCustom` for platform extensions. - Refactored message extraction logic from snapshots to improve handling of messages from persisted snapshots. - Removed the deprecated `loadCompactedMessages` function and updated the event streaming logic to utilize persisted message snapshots for better performance and reliability. These changes enhance the overall stability and functionality of the AG-UI event handling system.
Claude Code ReviewSummaryThis PR introduces a new capabilities endpoint and significantly refactors the AGUI event handling system. The changes replace custom event compaction logic with runner-emitted snapshots and integrate CopilotKit for the frontend chat UI. Overall, the implementation demonstrates strong security practices and architectural clarity, with a few areas requiring attention before merge. Key Changes:
Issues by Severity🚫 Blocker IssuesNone - No critical security or correctness issues that block merge. 🔴 Critical Issues1. Frontend Type Definitions Violate StandardsLocation: The codebase standard is to always use Problem: // Added in this PR - violates guidelines
interface Capabilities { ... }Fix Required: // Should be:
type Capabilities = { ... }Reference: CLAUDE.md lines 1141-1145, frontend-development.md lines 73-76 2. Missing Type Safety in Capabilities ResponseLocation: var result map[string]interface{}
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
log.Printf("Capabilities: Failed to decode response: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to parse runner response"})
return
}
c.JSON(http.StatusOK, result)Issues:
Recommendation:
Pattern: See error-handling.md lines 199-220 for proper error exposure patterns. 3. Large Dependency Additions Without JustificationLocation: Added Dependencies:
Impact:
Missing:
Recommendation:
🟡 Major Issues4. Fallback Capabilities Response May Hide ErrorsLocation: if err != nil {
log.Printf("Capabilities: Request failed: %v", err)
// Runner not ready — return minimal default
c.JSON(http.StatusOK, gin.H{
"framework": "unknown",
"agent_features": []interface{}{},
"platform_features": []interface{}{},
"file_system": false,
"mcp": false,
})
return
}Issue:
Recommendation: c.JSON(http.StatusServiceUnavailable, gin.H{
"error": "Runner not available",
"message": "Session is starting or runner is unavailable",
})Frontend can then show appropriate loading/error state. 5. Missing Error Context in LogsLocation: if eventType == types.EventTypeMessagesSnapshot {
go persistMessagesSnapshot(sessionID, event)
}Issue:
Recommendation: 6. Deleted Compaction Logic Without Migration PathLocation: Issue:
Questions:
Recommendation: 🔵 Minor Issues7. Frontend Component Missing Loading StatesLocation: Issue:
Recommendation: export function CopilotChatPanel({ projectName, sessionName }: Props) {
const { data: capabilities, isLoading, error } = useCapabilities(projectName, sessionName);
if (isLoading) return <div>Initializing chat...</div>;
if (error) return <div>Failed to connect: {error.message}</div>;
return <CopilotKit runtimeUrl={...}>...</CopilotKit>;
}Reference: frontend-development.md line 156 (all buttons/components need loading states) 8. Typo Fixed But Inconsistent NamingLocation: -EventTypStateDelta = "STATE_DELTA" // Typo fixed
+EventTypeStateDelta = "STATE_DELTA"Good: Typo fixed ✅ Issue: Existing code may reference grep -r "EventTypStateDelta" components/backend components/operator9. Missing Test Coverage for New EndpointLocation: Issue:
Recommendation: func TestHandleCapabilities_Unauthorized(t *testing.T) { ... }
func TestHandleCapabilities_RunnerUnavailable(t *testing.T) { ... }
func TestHandleCapabilities_Success(t *testing.T) { ... }10. Runner Endpoint Uses Global StateLocation: has_langfuse = state._obs is not None and state._obs.langfuse_client is not NoneIssue:
Recommendation: def has_observability() -> bool:
return state._obs is not None and state._obs.langfuse_client is not NonePositive Highlights✅ Security Done Right
Reference Compliance: Follows k8s-client-usage.md patterns exactly. ✅ ✅ Excellent Code Organization
✅ React Query UsageThe new
Reference Compliance: Follows react-query-usage.md patterns exactly. ✅ ✅ Backend Proxy PatternThe
✅ DocumentationAdded comprehensive ADR (0006-ambient-runner-sdk-architecture.md, +423 lines) explaining architectural decisions. Strong documentation culture. 📚 RecommendationsPriority 1 (Before Merge)
Priority 2 (Nice to Have)
Priority 3 (Future Work)
Overall AssessmentCode Quality: 8.5/10 — Strong security practices, good architectural clarity, minor issues with frontend standards compliance. Readiness: APPROVE with changes — Fix the frontend The removal of compaction logic is a significant architectural improvement (MESSAGES_SNAPSHOT from runner is cleaner). The new capabilities endpoint follows all security patterns correctly. Great work on the refactor! 🚀 References:
🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
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. |
/agentic-sessions/:sessionName/agui/capabilities.HandleCapabilitiesfunction to authenticate users, verify permissions, and proxy requests to the runner.CopilotChatPanelfor improved user experience.This update improves the overall functionality and performance of the AG-UI system, allowing for better integration with the runner's capabilities and enhancing user interactions.