fix(platform/copilot): bypass Vercel SSE proxy, refactor hook architecture#12210
fix(platform/copilot): bypass Vercel SSE proxy, refactor hook architecture#12210
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded timing and robustness to Copilot SDK streaming and transcript handling, introduced partial-message streaming support, tightened SSE heartbeat timeout, switched frontend SSE to direct backend with JWT auth and improved reconnect/dedup logic, and updated marker parsing to use escaped hex-suffixed prefixes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Client
participant Auth as Auth Service
participant Backend as Python Backend (SSE)
Note over Client,Backend: Direct SSE with JWT auth and reconnection
Client->>Auth: request fresh JWT (getWebSocketToken)
Auth-->>Client: JWT token
Client->>Backend: SSE request (Authorization: Bearer <token>)
Backend-->>Client: SSE stream events / heartbeats
loop Reconnect (exponential backoff)
Client->>Auth: request fresh JWT
Auth-->>Client: JWT token
Client->>Backend: resume SSE (Authorization header)
Backend-->>Client: continue stream
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
| // Only dedup if this assistant message is identical to the previous one | ||
| if (fingerprint && fingerprint === lastAssistantFingerprint) return false; | ||
| if (fingerprint) lastAssistantFingerprint = fingerprint; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Good catch — this was a real bug. Fixed in commit 1ad10bf: the fingerprint tracker now resets when a non-assistant message is encountered, so [assistant: "Done!", user: "thanks", assistant: "Done!"] correctly keeps both assistant messages.
| // | ||
| // This route is kept as a fallback / for backwards compatibility. | ||
| // See useCopilotPage.ts for the direct SSE transport implementation. | ||
| // --------------------------------------------------------------------------- |
Fixes false-positive deduplication when identical assistant responses are separated by user messages (e.g. "Done!" → user → "Done!"). The fingerprint tracker now resets when a non-assistant message is encountered, so only truly consecutive duplicates are filtered. Addresses Sentry bot review comment on PR #12210. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
- Connect directly to Python backend for SSE, bypassing the Next.js serverless proxy and its 800s Vercel function timeout - Add content-fingerprint deduplication that resets on non-assistant messages, preventing false removal of legitimately repeated responses - Throw on auth failure in getAuthHeaders() instead of silently returning empty headers (fixes Sentry HIGH severity) - Use refs instead of state for reconnect attempt counter to avoid stale closures in rapid reconnect cycles - Add hex-suffixed marker prefixes to prevent LLM false-positives - Mark SSE proxy route as legacy with fallback comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ff9437b to
3d172e7
Compare
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 6 conflict(s), 0 medium risk, 5 low risk (out of 11 PRs with file overlap) Auto-generated on push. Ignores: |
afd9ec9 to
2ecc588
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
255-269: Consider consistent case handling for auth error detection.The auth error checks mix case-sensitive and case-insensitive comparisons:
- Lines 258-260 use exact case matching
- Line 261 uses
.toLowerCase()for "401"This could miss lowercase variants like
"unauthorized"from some HTTP clients/backends.♻️ Optional: Consistent lowercase comparison
+ const lowerMessage = error.message.toLowerCase(); const isAuthError = - error.message.includes("Authentication failed") || - error.message.includes("Unauthorized") || - error.message.includes("Not authenticated") || - error.message.toLowerCase().includes("401"); + lowerMessage.includes("authentication failed") || + lowerMessage.includes("unauthorized") || + lowerMessage.includes("not authenticated") || + lowerMessage.includes("401");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts around lines 255 - 269, The auth error detection in isAuthError mixes case-sensitive checks with a single lowercased check, so replace the mixed comparisons by normalizing error.message (e.g., toLowerCase()) and then checking against lowercase strings ("authentication failed", "unauthorized", "not authenticated", "401") to ensure consistent matching; update the isAuthError logic in the useCopilotPage handler that builds the isAuthError variable (and keep the subsequent toast call for authentication errors) so all comparisons are case-insensitive and reliable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 255-269: The auth error detection in isAuthError mixes
case-sensitive checks with a single lowercased check, so replace the mixed
comparisons by normalizing error.message (e.g., toLowerCase()) and then checking
against lowercase strings ("authentication failed", "unauthorized", "not
authenticated", "401") to ensure consistent matching; update the isAuthError
logic in the useCopilotPage handler that builds the isAuthError variable (and
keep the subsequent toast call for authentication errors) so all comparisons are
case-insensitive and reliable.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
autogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.tsautogpt_platform/frontend/src/app/api/chat/sessions/[sessionId]/stream/route.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
- autogpt_platform/frontend/src/app/api/chat/sessions/[sessionId]/stream/route.ts
- autogpt_platform/backend/backend/copilot/sdk/service.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: types
- GitHub Check: Seer Code Review
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: Check PR Status
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Runpnpm formatto auto-fix formatting issues before completing work
Runpnpm lintto check for lint errors and fix any that appear before completing work
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend developmentRun
pnpm typesto check for type errors and fix any that appear before completing work
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}: Format frontend code usingpnpm format
Never use components fromsrc/components/__legacy__/*
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.tsand use design system components fromsrc/components/(atoms, molecules, organisms)
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}and regenerate withpnpm generate:api
Use function declarations (not arrow functions) for components and handlers
Separate render logic from business logic with component.tsx + useComponent.ts + helpers.ts structure
Colocate state when possible, avoid creating large components, use sub-components in local/componentsfolder
Avoid large hooks, abstract logic intohelpers.tsfiles when sensible
Use arrow functions only for callbacks, not for component declarations
Avoid comments at all times unless the code is very complex
Do not useuseCallbackoruseMemounless asked to optimize a given function
autogpt_platform/frontend/src/**/*.{ts,tsx}: Use function declarations (not arrow functions) for components and handlers
Use type-safe generated API hooks via Orval + React Query for data fetching
Use React Query for server state management and co-locate UI state in components/hooks
Separate render logic (.tsx) from business logic (use*.tshooks)
Use only shadcn/ui (Radix UI primitives) with Tailwind CSS for UI components
Use Phosphor Icons only for all icon implementations
Use ErrorCard component for render errors, toast for mutations, and Sentry for exceptions
Use design system components fromsrc/components/(atoms, molecules, organisms)
Never usesrc/components/__legacy__/*components
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Use Tailwind CSS only for styling with design tokens
Do not useuseCallbackoruseMemounless asked to optimize a specific function
Never type withanyunless a variable/attribute can actually be of any type
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS only for styling, use design tokens, and use Phosphor Icons only
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Fully capitalize acronyms in symbols, e.g.
graphID,useBackendAPI
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/use*.ts
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
autogpt_platform/frontend/src/**/use*.ts: Extract component logic into custom hooks grouped by concern, with each hook in its own.tsfile
Do not type hook returns; let TypeScript infer types as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
🧠 Learnings (4)
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{tsx,ts} : Use generated API hooks from '@/app/api/__generated__/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/*'
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/src/lib/supabase/middleware.ts : Update 'frontend/lib/supabase/middleware.ts' when adding protected routes in frontend
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:50:51.495Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T16:50:51.495Z
Learning: Applies to autogpt_platform/frontend/src/app/(platform)/**/*.tsx : If adding protected frontend routes, update `frontend/lib/supabase/middleware.ts`
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{ts,tsx} : Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
🧬 Code graph analysis (1)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (2)
autogpt_platform/frontend/src/services/environment/index.ts (1)
environment(140-166)autogpt_platform/frontend/src/lib/supabase/actions.ts (1)
getWebSocketToken(105-145)
🔇 Additional comments (6)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (6)
22-29: LGTM!The reduced
RECONNECT_MAX_ATTEMPTSto 3 is reasonable for faster failure feedback. ThegetBackendBaseUrl()helper correctly strips the/apisuffix to construct the SSE endpoint path.
31-39: LGTM!Clean implementation that mirrors the WebSocket token pattern. The error message thrown here correctly matches the auth error detection in
onError(line 258).
68-97: LGTM!The revised deduplication strategy is well-designed. By comparing only consecutive assistant messages, it correctly handles:
- Hydration/stream boundary duplicates (same content, different IDs)
- Legitimate repeated responses separated by user messages
The empty fingerprint guard on line 87 (
fingerprint &&) safely handles edge cases with empty or non-text parts.
181-223: LGTM!The switch from
useStatetouseRefforreconnectAttemptsRefcorrectly addresses stale closure reads during rapid reconnect cycles. The backoff calculation2 ** (nextAttempt - 1)produces the expected progression: 1s → 2s → 4s for attempts 1-3.
331-358: LGTM!The
reconnectAttemptsRefresets are correctly placed:
- Line 334: Clears on session switch to prevent stale counts
- Line 354: Clears after successful stream completion to allow fresh reconnect attempts
153-179: Async callbacks are supported by DefaultChatTransport.The
aiSDK explicitly supportsPromiseLikereturn types forprepareSendMessagesRequestand similar prepare methods, so the async calls togetAuthHeaders()work as intended.
…switch resume - Enable `include_partial_messages` in SDK options for token-by-token streaming; add StreamEvent handler in SDKResponseAdapter to convert content_block deltas to StreamTextStart/Delta/End - Reduce heartbeat interval 10s→3s and queue timeout 30s→10s for faster stall detection - Move stream lock release to top of finally block so users can send follow-up messages immediately (before slow persist/upload) - Add transcript_download_timeout (3s) to prevent GCS hangs in local dev - Add timing instrumentation throughout SDK startup path - Pre-compile marker regexes to module-level constants - Use getAGPTServerBaseUrl() instead of custom getBackendBaseUrl() - Fix stale isReconnectScheduled via ref mirror - Clear hasResumedRef on session switch so returning to a session with an active stream auto-reconnects Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/service.py`:
- Around line 1117-1122: The unguarded await lock.release() in the finally block
can raise CancelledError and abort the subsequent shielded persistence and
transcript upload; change the release to be cancellation-safe by shielding or
catching cancellation (e.g., use asyncio.shield on lock.release() or wrap await
lock.release() in a try/except asyncio.CancelledError and then call
asyncio.shield(lock.release())/re-run release) so that the shielded persistence
and transcript upload always run; update the code around lock.release() in the
finally block (the await lock.release() call) to use asyncio.shield or a
cancellation-aware pattern so downstream persistence/transcript upload always
executes.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 68-80: The existing dedup logic in useCopilotPage builds a
fingerprint from msg.parts using only text and toolCallId, causing messages with
the same toolCallId but different tool state to be dropped; update the
fingerprint computation (used where msg.role === "assistant" and compared to
lastAssistantFingerprint) to include relevant tool-state fields such as any
part.state, part.output, and part.errorText (in addition to text and toolCallId)
so that changes to tool state produce a different fingerprint and won’t be
filtered out.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
autogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.pyautogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Seer Code Review
- GitHub Check: types
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: Check PR Status
- GitHub Check: Analyze (python)
- GitHub Check: end-to-end tests
🧰 Additional context used
📓 Path-based instructions (22)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Runpnpm formatto auto-fix formatting issues before completing work
Runpnpm lintto check for lint errors and fix any that appear before completing work
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend developmentRun
pnpm typesto check for type errors and fix any that appear before completing work
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}: Format frontend code usingpnpm format
Never use components fromsrc/components/__legacy__/*
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.tsand use design system components fromsrc/components/(atoms, molecules, organisms)
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}and regenerate withpnpm generate:api
Use function declarations (not arrow functions) for components and handlers
Separate render logic from business logic with component.tsx + useComponent.ts + helpers.ts structure
Colocate state when possible, avoid creating large components, use sub-components in local/componentsfolder
Avoid large hooks, abstract logic intohelpers.tsfiles when sensible
Use arrow functions only for callbacks, not for component declarations
Avoid comments at all times unless the code is very complex
Do not useuseCallbackoruseMemounless asked to optimize a given function
autogpt_platform/frontend/src/**/*.{ts,tsx}: Use function declarations (not arrow functions) for components and handlers
Use type-safe generated API hooks via Orval + React Query for data fetching
Use React Query for server state management and co-locate UI state in components/hooks
Separate render logic (.tsx) from business logic (use*.tshooks)
Use only shadcn/ui (Radix UI primitives) with Tailwind CSS for UI components
Use Phosphor Icons only for all icon implementations
Use ErrorCard component for render errors, toast for mutations, and Sentry for exceptions
Use design system components fromsrc/components/(atoms, molecules, organisms)
Never usesrc/components/__legacy__/*components
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Use Tailwind CSS only for styling with design tokens
Do not useuseCallbackoruseMemounless asked to optimize a specific function
Never type withanyunless a variable/attribute can actually be of any type
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS only for styling, use design tokens, and use Phosphor Icons only
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Component props should be
interface Props { ... }(not exported) unless the interface needs to be used outside the componentUse
type Props = { ... }(not exported) for component props unless used outside the component
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/app/(platform)/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
If adding protected frontend routes, update
frontend/lib/supabase/middleware.ts
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
autogpt_platform/frontend/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Fully capitalize acronyms in symbols, e.g.
graphID,useBackendAPI
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Put sub-components in a local
components/folder within the feature directory
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
autogpt_platform/frontend/src/**/[A-Z]*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Structure components as ComponentName/ComponentName.tsx + useComponentName.ts + helpers.ts
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.py
autogpt_platform/backend/backend/api/features/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
When modifying API routes, update corresponding Pydantic models in the same directory and write tests alongside the route file
Files:
autogpt_platform/backend/backend/api/features/chat/routes.py
autogpt_platform/backend/backend/api/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/backend/api/**/*.py: Use FastAPI for building REST and WebSocket endpoints
Use JWT-based authentication with Supabase integration
Files:
autogpt_platform/backend/backend/api/features/chat/routes.py
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/use*.ts
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
autogpt_platform/frontend/src/**/use*.ts: Extract component logic into custom hooks grouped by concern, with each hook in its own.tsfile
Do not type hook returns; let TypeScript infer types as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
🧠 Learnings (8)
📚 Learning: 2026-02-26T10:12:58.845Z
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12207
File: autogpt_platform/frontend/src/components/ai-elements/conversation.tsx:0-0
Timestamp: 2026-02-26T10:12:58.845Z
Learning: Guideline: Do not apply dark mode CSS classes (e.g., dark:text-*) to copilot UI components until dark mode support is implemented. Applies to all copilot-related components (paths containing /copilot/). When reviewing, search for dark:* class names within copilot components and refactor to use conditional class sets or feature-flag gates, ensuring no dark-mode styles are present in the code paths that render copilot UI unless dark mode support is officially enabled.
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
📚 Learning: 2026-02-27T10:45:49.499Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/(platform)/copilot/tools/RunMCPTool/helpers.tsx:23-24
Timestamp: 2026-02-27T10:45:49.499Z
Learning: Prefer using generated OpenAPI types from '@/app/api/__generated__/' for payloads defined in openapi.json (e.g., MCPToolsDiscoveredResponse, MCPToolOutputResponse). Use inline TypeScript interfaces only for payloads that are SSE-stream-only and not exposed via OpenAPI. Apply this pattern to frontend tool components (e.g., RunMCPTool) and related areas where similar SSE/openapi-discrepancies occur; avoid re-implementing types when a generated type is available.
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
📚 Learning: 2026-02-27T15:58:44.424Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12213
File: autogpt_platform/frontend/src/app/api/openapi.json:9983-9995
Timestamp: 2026-02-27T15:58:44.424Z
Learning: Repo: Significant-Gravitas/AutoGPT PR: 12213 — OpenAPI/codegen
Learning: Ensuring a field is required in generated TS types needs two sides: (1) no default value on the Pydantic field, and (2) the OpenAPI model's "required" array must list it. For MCPToolInfo, making input_schema required in OpenAPI and removing Field(default_factory=dict) in the backend prevents optional typing drift.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.pyautogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/response_adapter.py
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{tsx,ts} : Use generated API hooks from '@/app/api/__generated__/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/*'
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/src/lib/supabase/middleware.ts : Update 'frontend/lib/supabase/middleware.ts' when adding protected routes in frontend
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{ts,tsx} : Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📚 Learning: 2026-02-04T16:50:51.495Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T16:50:51.495Z
Learning: Applies to autogpt_platform/frontend/src/app/(platform)/**/*.tsx : If adding protected frontend routes, update `frontend/lib/supabase/middleware.ts`
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
🧬 Code graph analysis (3)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (2)
autogpt_platform/frontend/src/lib/supabase/actions.ts (1)
getWebSocketToken(105-145)autogpt_platform/frontend/src/services/environment/index.ts (1)
environment(140-166)
autogpt_platform/backend/backend/copilot/sdk/service.py (2)
autogpt_platform/backend/backend/copilot/sdk/transcript.py (1)
download_transcript(402-453)autogpt_platform/backend/backend/executor/cluster_lock.py (2)
release(118-130)release(239-251)
autogpt_platform/backend/backend/copilot/sdk/response_adapter.py (1)
autogpt_platform/backend/backend/copilot/response_model.py (3)
StreamBaseResponse(47-55)StreamStartStep(86-93)StreamTextDelta(116-121)
🔇 Additional comments (7)
autogpt_platform/backend/backend/copilot/config.py (1)
95-99: Config addition is clean and correctly scoped.Making transcript download timeout configurable here is a good, low-risk change that supports the resilience goal.
autogpt_platform/backend/backend/api/features/chat/routes.py (1)
490-490: Timeout tightening looks appropriate for stream stall recovery.Line [490] and Line [643] reduce queue wait latency while preserving heartbeat behavior, which matches the PR’s robustness intent.
Also applies to: 643-643
autogpt_platform/backend/backend/copilot/sdk/sdk_compat_test.py (1)
108-108: Good compatibility coverage update.Including
include_partial_messagesin the asserted option surface is the right guard for this PR’s partial-streaming dependency.autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx (1)
34-55: Marker parsing hardening is solid.Escaped, precompiled marker regexes with distinct hex-suffixed prefixes are a good reliability improvement for streaming text parsing.
Also applies to: 62-63, 71-72
autogpt_platform/backend/backend/copilot/sdk/service.py (1)
605-617: Transcript timeout handling is a good resilience improvement.The explicit timeout + fallback on download avoids long hangs when storage is unreachable and keeps stream startup predictable.
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
23-31: Direct backend transport and ref-based reconnect guards are well implemented.This is a strong reliability upgrade: explicit JWT headers for backend SSE plus ref-based reconnect state avoids stale-closure reconnect bugs.
Also applies to: 145-167, 173-177
autogpt_platform/backend/backend/copilot/sdk/response_adapter.py (1)
22-23: StreamEvent support is integrated cleanly with existing adapter state.The event-to-protocol mapping and post-Assistant reset logic are solid, and the duplicate-text guard is a practical fit for partial-message streaming.
Also applies to: 60-64, 83-85, 104-107, 133-135, 249-291
| # Release stream lock FIRST to allow new streams for this session. | ||
| # Session persist and transcript upload can be slow — releasing the | ||
| # lock early prevents "Another stream is already active" errors when | ||
| # the user sends a new message immediately after a stream completes. | ||
| await lock.release() | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C5 'finally:|await lock\.release\(|asyncio\.shield\(upsert_chat_session|asyncio\.shield\(_try_upload_transcript' autogpt_platform/backend/backend/copilot/sdk/service.pyRepository: Significant-Gravitas/AutoGPT
Length of output: 2650
🏁 Script executed:
cd autogpt_platform/backend/backend/copilot/sdk && grep -n "CancelledError\|shield\|finally:" service.py | head -20Repository: Significant-Gravitas/AutoGPT
Length of output: 1052
Guard await lock.release() to prevent cancellation from skipping shielded cleanup.
At line 1121, await lock.release() is unguarded and the first awaited operation in the finally block. A CancelledError at this point will immediately propagate and skip the subsequent shielded persistence and transcript upload operations, risking message and transcript loss on interrupted streams.
Suggested fix
finally:
- await lock.release()
+ try:
+ await lock.release()
+ except asyncio.CancelledError:
+ logger.warning(
+ "[SDK] [%s] Cancelled during lock release; continuing cleanup",
+ session_id[:12],
+ )
+ release_cancelled = True
+ except Exception as release_err:
+ logger.warning(
+ "[SDK] [%s] Failed to release stream lock: %s",
+ session_id[:12],
+ release_err,
+ exc_info=True,
+ )
# --- Persist session messages ---
if session is not None:
try:
await asyncio.shield(upsert_chat_session(session))Add at end of finally block:
+ if release_cancelled:
+ raise asyncio.CancelledError🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/copilot/sdk/service.py` around lines 1117 -
1122, The unguarded await lock.release() in the finally block can raise
CancelledError and abort the subsequent shielded persistence and transcript
upload; change the release to be cancellation-safe by shielding or catching
cancellation (e.g., use asyncio.shield on lock.release() or wrap await
lock.release() in a try/except asyncio.CancelledError and then call
asyncio.shield(lock.release())/re-run release) so that the shielded persistence
and transcript upload always run; update the code around lock.release() in the
finally block (the await lock.release() call) to use asyncio.shield or a
cancellation-aware pattern so downstream persistence/transcript upload always
executes.
| if (msg.role === "assistant") { | ||
| const fingerprint = msg.parts | ||
| .map( | ||
| (p) => | ||
| ("text" in p && p.text) || | ||
| ("toolCallId" in p && p.toolCallId) || | ||
| "", | ||
| ) | ||
| .join("|"); | ||
|
|
||
| // Only dedup if this assistant message is identical to the previous one | ||
| if (fingerprint && fingerprint === lastAssistantFingerprint) return false; | ||
| if (fingerprint) lastAssistantFingerprint = fingerprint; |
There was a problem hiding this comment.
Dedup fingerprint can drop legitimate tool-state updates.
At Line [69]-Line [75], the fingerprint uses only text/toolCallId. If two consecutive assistant messages share the same toolCallId but differ in state, output, or errorText, the newer message can be incorrectly filtered out.
💡 Proposed fix
- const fingerprint = msg.parts
- .map(
- (p) =>
- ("text" in p && p.text) ||
- ("toolCallId" in p && p.toolCallId) ||
- "",
- )
- .join("|");
+ const fingerprint = msg.parts.map((p) => JSON.stringify(p)).join("|");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (msg.role === "assistant") { | |
| const fingerprint = msg.parts | |
| .map( | |
| (p) => | |
| ("text" in p && p.text) || | |
| ("toolCallId" in p && p.toolCallId) || | |
| "", | |
| ) | |
| .join("|"); | |
| // Only dedup if this assistant message is identical to the previous one | |
| if (fingerprint && fingerprint === lastAssistantFingerprint) return false; | |
| if (fingerprint) lastAssistantFingerprint = fingerprint; | |
| if (msg.role === "assistant") { | |
| const fingerprint = msg.parts.map((p) => JSON.stringify(p)).join("|"); | |
| // Only dedup if this assistant message is identical to the previous one | |
| if (fingerprint && fingerprint === lastAssistantFingerprint) return false; | |
| if (fingerprint) lastAssistantFingerprint = fingerprint; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts
around lines 68 - 80, The existing dedup logic in useCopilotPage builds a
fingerprint from msg.parts using only text and toolCallId, causing messages with
the same toolCallId but different tool state to be dropped; update the
fingerprint computation (used where msg.role === "assistant" and compared to
lastAssistantFingerprint) to include relevant tool-state fields such as any
part.state, part.output, and part.errorText (in addition to text and toolCallId)
so that changes to tool state produce a different fingerprint and won’t be
filtered out.
When switching sessions, useChat destroys the Chat instance, losing all messages. Recovery depends on REST hydration + stream resume, but the target session's query cache could be stale (cached before the user sent their last message), causing hasActiveStream to be false and the resume effect to never fire — leaving the user with an empty chat and a stuck stop button. Fix: invalidate both the source AND target session queries on switch, guaranteeing a fresh fetch with accurate active_stream state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
…d hook into domain modules Backend: revert partial streaming, timing instrumentation, transcript timeout, and lock ordering changes — keep only heartbeat (3s) and hex-suffixed marker prefixes needed for direct browser SSE. Frontend: decompose the 490-line useCopilotPage into focused modules: - helpers.ts: pure functions (deduplicateMessages, resolveInProgressTools) - store.ts: Zustand store for shared UI state (sessionToDelete, drawer) - useCopilotStream.ts: SSE transport, useChat, reconnect, resume, stop - useCopilotPage.ts: thin orchestrator (~160 lines) - ChatSidebar: reads sessionToDelete from store instead of duplicating Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
autogpt-reviewer
left a comment
There was a problem hiding this comment.
PR #12210 — fix(platform/copilot): bypass Vercel SSE proxy, refactor hook architecture
Author: 0ubbe | Re-review #2 (f448f66 → e6e1bdf) | Files: 10 (+487/-308)
🎯 Verdict: APPROVE
What This PR Does
Switches copilot SSE streaming from a Next.js serverless proxy (subject to Vercel's 800s function timeout) to a direct browser→Python backend connection with JWT auth. Decomposes the 490-line useCopilotPage monolith into focused domain modules: helpers.ts (pure functions), store.ts (Zustand shared UI state), useCopilotStream.ts (SSE transport/reconnect), and a thin useCopilotPage.ts orchestrator. Also hardens marker prefixes with hex suffixes to prevent LLM false-positives, fixes stale closure bugs in reconnect logic, and improves auth error handling with user-facing toasts.
Previous Blockers — ALL RESOLVED ✅
| Blocker | Status | Evidence |
|---|---|---|
| 🔴 CORS not configured for direct SSE | ✅ Resolved | ntindle (maintainer) confirmed this was a local setup issue, not a code bug. Backend CORSMiddleware correctly uses backend_cors_allow_origins setting. QA confirms zero CORS errors. |
🔴 isReconnectScheduled stale closure → infinite reconnect |
✅ Fixed | Now uses dual ref+state pattern (useCopilotStream.ts:78-83). QA confirmed no reconnect loops. |
Specialist Findings
🛡️ Security ✅ — JWT auth follows existing WebSocket pattern. Token fetched via server action (not stored client-side). Backend enforces auth via Depends(auth.get_user_id). Marker hex suffixes reduce LLM false-positives. No info leakage in error messages. Zustand store holds UI-only state.
🏗️ Architecture ✅ — Clean hook decomposition with unidirectional data flow (useChatSession → useCopilotStream → useCopilotPage). No circular dependencies. getBackendBaseUrl() duplication eliminated — now uses environment.getAGPTServerBaseUrl(). Server action for auth is architecturally sound (one-shot RPC, not subject to timeout). CORS handled by existing middleware. Legacy proxy route preserved as fallback.
⚡ Performance ✅ — SSE proxy elimination is a net latency win. Regex pre-compiled at module level (fixed from v1). deduplicateMessages is O(n), bounded by conversation length. Heartbeat 10s→3s acceptable (negligible bandwidth, monitor at scale). One should-fix: JWT fetched per-request via server action (~50-150ms overhead) — should cache client-side.
🧪 Testing helpers.ts has two pure functions (deduplicateMessages, resolveInProgressTools) that are trivially unit-testable. useCopilotStream.ts (300 LOC) contains the reconnect logic that was the source of the previous blocker bug — high-risk untested area. Auth error string matching is fragile and untested. CI test(3.11) failure is pre-existing flaky run_agent_test — unrelated.
📖 Quality ✅ — Well-structured decomposition. No any types. Consistent naming conventions. Function declarations used per project convention. 3 should-fixes: (1) isUserStoppingRef leaked as mutable ref across hook boundary — should expose callback instead, (2) unsafe type assertion on refetchSession result, (3) DeleteTarget.title typed as string | null | undefined — normalize to string | null.
📦 Product ✅ — SSE proxy bypass solves the core Vercel timeout pain point. Consecutive-only dedup correctly preserves legitimate repeated responses. Session switch auto-reconnect works. 2 should-fixes: (1) auth error toast has no action button/redirect to login, (2) marker prefix change breaks backward compatibility for existing conversations stored with old format.
📬 Discussion ✅ — Both original blockers resolved. 3/7 CodeRabbit findings fixed, 4 remaining are nitpick-level. All 3 Sentry findings addressed. majdyz APPROVED latest commit (informed reviewer — author of related PR #12205). CI effectively green. 6 PRs have merge conflicts — recommend merging this first since the refactoring makes old useCopilotPage.ts conflicts moot.
🔎 QA ✅ — Full QA pass. SSE streaming works end-to-end including tool execution. Zero CORS errors. No reconnect loops. Session switching works cleanly with no message duplication. Sign up, login, copilot chat, new chat creation all functional.
QA Screenshots
- Landing
- Dashboard
- Copilot page
- First message (SSE streaming)
- Tool execution (PerplexityBlock)
- Tool result complete
- New chat
- Session switch
Should Fix (Follow-up OK)
helpers.ts— Add unit tests fordeduplicateMessagesandresolveInProgressTools— Pure functions, trivially testable, high value. Dedup logic is non-trivial and the reconnect area was the source of the previous blocker.useCopilotStream.ts:19-26— Cache JWT client-side —getAuthHeaders()calls server action per-request (~50-150ms). Supabase tokens valid ~1hr. Cache with TTL or use client-sidesupabase.auth.getSession().ChatMessagesContainer.tsx:34-35— Support old marker prefixes for backward compat — Marker change from[COPILOT_ERROR]→[__COPILOT_ERROR_f7a1__]breaks parsing of existing stored conversations. Add regex alternation for both formats.useCopilotStream.ts— ExposeresetStopFlag()callback instead of leakingisUserStoppingRef— Mutable ref crossing hook boundary is fragile.store.ts:5— NormalizeDeleteTarget.titletostring | null—string | null | undefinedis a code smell.useCopilotStream.ts:152-157— Consistent case-insensitive auth error matching — Currently mixes case-sensitive and case-insensitive checks.helpers.ts:42-44— Include tool state in dedup fingerprint — Current fingerprint uses onlytext/toolCallId, may drop messages where tool state changed but ID matches.- No feature flag for SSE transport — Consider
NEXT_PUBLIC_USE_DIRECT_SSEenv var to toggle between direct and proxy modes at runtime.
Risk Assessment
Merge risk: LOW | Rollback: EASY (revert to proxy route, which is preserved as legacy)
@ntindle Both previous blockers resolved — CORS confirmed as setup issue, stale closure fixed with ref+state pattern. QA passes clean with SSE streaming, tool execution, and session switching all working. Architecture is a significant improvement (490-line monolith → 4 focused modules). Approved by majdyz. Zero code blockers — recommend merge. Note: 6 PRs have conflicts; merging this first is optimal since the refactoring obsoletes old useCopilotPage.ts conflict targets.
…11 event loop stability On Python 3.11, the httpx transport inside Prisma can reference a stale (closed) event loop when session-scoped async fixtures are evaluated long after the initial server fixture connected Prisma. This caused all 9 run_agent_test.py tests using setup_test_data to fail with "Event loop is closed" on the 3.11 CI job. Add _ensure_db_connected() health check at the start of each test data fixture that does a cheap SELECT 1 and reconnects if the connection is stale. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Continue on #12254 |
Summary
Reliability and architecture improvements for the CoPilot SSE streaming pipeline.
Frontend
useCopilotPagemonolith into focused domain modules:helpers.ts— pure functions (deduplicateMessages,resolveInProgressTools)store.ts— Zustand store for shared UI state (sessionToDelete, drawer open/close)useCopilotStream.ts— SSE transport,useChatwrapper, reconnect/resume logic, stop+canceluseCopilotPage.ts— thin orchestrator (~160 lines)sessionToDeletefrom Zustand store instead of duplicating delete state/mutation locallygetAuthHeaders()throws on failure instead of silently returning empty headers; 401 errors show user-facing toasthasResumedRefon session switch so returning to a session with an active stream auto-reconnectsactive_streamis accurate for resume[__COPILOT_ERROR_f7a1__]) to prevent LLM false-positivesBackend (minimal)
Test plan
pnpm format && pnpm lint && pnpm typespass🤖 Generated with Claude Code