-
Notifications
You must be signed in to change notification settings - Fork 92
refactor(frontend): 修复刷新页面后流式消息丢失的问题,修复会话页面偶发白屏的问题 #654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||||||||
| // SPDX-FileCopyrightText: 2025 Weibo, Inc. | ||||||||||||||
| // | ||||||||||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||||||||||
|
|
||||||||||||||
| 'use client' | ||||||||||||||
|
|
||||||||||||||
| import { useEffect } from 'react' | ||||||||||||||
| import { useTranslation } from '@/hooks/useTranslation' | ||||||||||||||
| import { Button } from '@/components/ui/button' | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Next.js error boundary for the chat route. | ||||||||||||||
| * Catches rendering errors in ChatPageDesktop/ChatPageMobile and shows | ||||||||||||||
| * a recovery UI instead of a blank white screen. | ||||||||||||||
| */ | ||||||||||||||
| export default function ChatError({ | ||||||||||||||
| error, | ||||||||||||||
| reset, | ||||||||||||||
| }: { | ||||||||||||||
| error: Error & { digest?: string } | ||||||||||||||
| reset: () => void | ||||||||||||||
| }) { | ||||||||||||||
| const { t } = useTranslation('common') | ||||||||||||||
|
|
||||||||||||||
| useEffect(() => { | ||||||||||||||
| console.error('[ChatError] Caught rendering error:', error) | ||||||||||||||
| }, [error]) | ||||||||||||||
|
|
||||||||||||||
| return ( | ||||||||||||||
| <div className="flex h-full w-full items-center justify-center bg-base"> | ||||||||||||||
| <div className="flex flex-col items-center gap-4 text-center"> | ||||||||||||||
| <p className="text-sm text-text-secondary">{t('errors.request_failed')}</p> | ||||||||||||||
| <Button variant="primary" onClick={reset}> | ||||||||||||||
| {t('actions.retry')} | ||||||||||||||
| </Button> | ||||||||||||||
|
Comment on lines
+33
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure retry button meets mobile tap-target size requirement. At Line [33], the button does not explicitly enforce the required 44×44 minimum target size. 📱 Suggested fix- <Button variant="primary" onClick={reset}>
+ <Button variant="primary" onClick={reset} className="h-11 min-w-[44px]">
{t('actions.retry')}
</Button>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| </div> | ||||||||||||||
| </div> | ||||||||||||||
| ) | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import { useTranslation } from '@/hooks/useTranslation' | |
| import { useRouter } from 'next/navigation' | ||
| import { useTaskContext } from '../../contexts/taskContext' | ||
| import { useTaskStateMachine } from '../../hooks/useTaskStateMachine' | ||
| import { useHasMessages } from '../../hooks/useHasMessages' | ||
| import { Button } from '@/components/ui/button' | ||
| import { useScrollManagement } from '../hooks/useScrollManagement' | ||
| import { useFloatingInput } from '../hooks/useFloatingInput' | ||
|
|
@@ -97,7 +98,8 @@ function ChatAreaContent({ | |
| const { quote, clearQuote, formatQuoteForMessage } = useQuote() | ||
|
|
||
| // Task context | ||
| const { selectedTaskDetail, setSelectedTask, accessDenied } = useTaskContext() | ||
| const { selectedTask, selectedTaskDetail, setSelectedTask, accessDenied, clearAccessDenied } = | ||
| useTaskContext() | ||
|
|
||
| // Use useTaskStateMachine hook for reactive state updates (SINGLE SOURCE OF TRUTH per AGENTS.md) | ||
| const { state: taskState } = useTaskStateMachine(selectedTaskDetail?.id) | ||
|
|
@@ -499,37 +501,13 @@ function ChatAreaContent({ | |
| teams: [...filteredTeams, ...teams], | ||
| }) | ||
|
|
||
| // Determine if there are messages to display (full computation) | ||
| // Note: Now using taskState.messages from state machine instead of selectedTaskDetail.subtasks | ||
| const hasMessages = useMemo(() => { | ||
| const hasSelectedTask = selectedTaskDetail && selectedTaskDetail.id | ||
| const hasNewTaskStream = | ||
| !selectedTaskDetail?.id && streamHandlers.pendingTaskId && streamHandlers.isStreaming | ||
| const hasLocalPending = streamHandlers.localPendingMessage !== null | ||
| // Use taskState from state machine (single source of truth) | ||
| const hasUnifiedMessages = taskState?.messages && taskState.messages.size > 0 | ||
|
|
||
| // If we have a selected task with messages in state machine, show messages | ||
| if (hasSelectedTask && hasUnifiedMessages) { | ||
| return true | ||
| } | ||
|
|
||
| return Boolean( | ||
| hasSelectedTask || | ||
| streamHandlers.hasPendingUserMessage || | ||
| streamHandlers.isStreaming || | ||
| hasNewTaskStream || | ||
| hasLocalPending || | ||
| hasUnifiedMessages | ||
| ) | ||
| }, [ | ||
| // Determine if there are messages to display | ||
| const hasMessages = useHasMessages({ | ||
| selectedTask, | ||
| selectedTaskDetail, | ||
| streamHandlers.hasPendingUserMessage, | ||
| streamHandlers.isStreaming, | ||
| streamHandlers.pendingTaskId, | ||
| streamHandlers.localPendingMessage, | ||
| taskState?.messages, | ||
| ]) | ||
| taskState, | ||
| streamHandlers, | ||
| }) | ||
|
Comment on lines
+504
to
+510
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please make render + hooks consume one consistent visibility source. 🤖 Prompt for AI Agents |
||
|
|
||
| // Note: Team selection is now handled by useTeamSelection hook in TeamSelector component | ||
| // Model selection is handled by useModelSelection hook in ModelSelector component | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,9 @@ export const TaskContextProvider = ({ children }: { children: ReactNode }) => { | |
| // Track task status for notification | ||
| const taskStatusMapRef = useRef<Map<number, TaskStatus>>(new Map()) | ||
|
|
||
| // AbortController for cancelling stale getTaskDetail requests during rapid task switching | ||
| const abortControllerRef = useRef<AbortController | null>(null) | ||
|
|
||
| // WebSocket connection for real-time task updates | ||
| const { registerTaskHandlers, isConnected, leaveTask, joinTask, onReconnect } = useSocket() | ||
|
|
||
|
|
@@ -688,18 +691,27 @@ export const TaskContextProvider = ({ children }: { children: ReactNode }) => { | |
| return | ||
| } | ||
|
|
||
| // Abort previous in-flight request to prevent stale responses overwriting current data | ||
| if (abortControllerRef.current) { | ||
| abortControllerRef.current.abort() | ||
| } | ||
| const controller = new AbortController() | ||
| abortControllerRef.current = controller | ||
|
|
||
| try { | ||
| // Clear access denied state before fetching | ||
| setAccessDenied(false) | ||
| // Fetch task metadata only (subtasks are now obtained via WebSocket task:join) | ||
| const updatedTaskDetail = await taskApis.getTaskDetail(selectedTask.id) | ||
| const updatedTaskDetail = await taskApis.getTaskDetail(selectedTask.id, controller.signal) | ||
|
|
||
| // Note: Workbench data extraction from subtasks is no longer needed here | ||
| // Subtasks are now managed by TaskStateMachine via WebSocket join response | ||
| // Workbench data should be obtained from the state machine or WebSocket events | ||
| // Verify the selected task hasn't changed while the request was in flight | ||
| if (controller.signal.aborted) return | ||
|
|
||
| setSelectedTaskDetail(updatedTaskDetail) | ||
| } catch (error) { | ||
| // Ignore abort errors - they are expected when switching tasks rapidly | ||
| if (error instanceof DOMException && error.name === 'AbortError') return | ||
|
|
||
| // Check if it's a 403 Forbidden or 404 Not Found error (access denied or task not found) | ||
| // Both cases should show the access denied UI to prevent information leakage | ||
| if (error instanceof ApiError && (error.status === 403 || error.status === 404)) { | ||
|
|
@@ -722,6 +734,8 @@ export const TaskContextProvider = ({ children }: { children: ReactNode }) => { | |
| // Leave previous task room if switching to a different task | ||
| if (previousTaskId !== null && previousTaskId !== currentTaskId) { | ||
| leaveTask(previousTaskId) | ||
| // Clear stale detail immediately so downstream hooks don't operate on wrong task data | ||
| setSelectedTaskDetail(null) | ||
| } | ||
|
Comment on lines
+737
to
739
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get overall file size and structure
wc -l frontend/src/features/tasks/contexts/taskContext.tsx
echo "---"
# Find all abort and setSelectedTaskDetail patterns
rg -n 'abortControllerRef|setSelectedTaskDetail' frontend/src/features/tasks/contexts/taskContext.tsxRepository: wecode-ai/Wegent Length of output: 670 Abort in-flight detail fetch when deselecting task or clearing selection. When Abort the controller before clearing detail in both the task-switch path and the deselect-to-null path: 🔧 Proposed fix if (previousTaskId !== null && previousTaskId !== currentTaskId) {
+ if (abortControllerRef.current) {
+ abortControllerRef.current.abort()
+ abortControllerRef.current = null
+ }
leaveTask(previousTaskId)
// Clear stale detail immediately so downstream hooks don't operate on wrong task data
setSelectedTaskDetail(null)
}
@@
if (selectedTask) {
@@
} else {
+ if (abortControllerRef.current) {
+ abortControllerRef.current.abort()
+ abortControllerRef.current = null
+ }
setSelectedTaskDetail(null)
}🤖 Prompt for AI Agents |
||
|
|
||
| // Update the ref to track current task | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // SPDX-FileCopyrightText: 2025 Weibo, Inc. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { useCallback } from 'react' | ||
|
|
||
| /** | ||
| * Cleans up selected text by: | ||
| * - Replacing 2+ consecutive newlines with a single newline | ||
| * - Trimming leading/trailing whitespace | ||
| */ | ||
| export function cleanCopyText(text: string): string { | ||
| return text.replace(/\n{2,}/g, '\n').trim() | ||
| } | ||
|
Comment on lines
+12
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Line 13 collapses all multi-newline spacing and trims both ends, which can degrade copied markdown/code readability. Keep intentional paragraph/code spacing while still removing accidental wrapper noise. Proposed fix export function cleanCopyText(text: string): string {
- return text.replace(/\n{2,}/g, '\n').trim()
+ return text
+ .replace(/\r\n/g, '\n')
+ // Keep one blank line between blocks; only collapse excessive gaps
+ .replace(/\n{3,}/g, '\n\n')
+ // Remove only boundary newlines introduced by wrapper elements
+ .replace(/^\n+|\n+$/g, '')
}🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Hook to handle copy events with text cleanup. | ||
| * Fixes the issue where block-level elements create extra newlines when copying. | ||
| */ | ||
| export function useCopyCleanup() { | ||
| const handleCopy = useCallback((e: React.ClipboardEvent) => { | ||
| const selection = window.getSelection() | ||
| if (!selection || selection.rangeCount === 0) return | ||
|
|
||
| const text = cleanCopyText(selection.toString()) | ||
| e.clipboardData.setData('text/plain', text) | ||
| e.preventDefault() | ||
| }, []) | ||
|
|
||
| return handleCopy | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // SPDX-FileCopyrightText: 2025 Weibo, Inc. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { useMemo } from 'react' | ||
| import type { TaskDetail } from '../../../types/api' | ||
| import type { TaskStateData } from '../state' | ||
|
|
||
| interface StreamHandlersState { | ||
| hasPendingUserMessage: boolean | ||
| isStreaming: boolean | ||
| pendingTaskId: number | null | ||
| localPendingMessage: unknown | null | ||
| } | ||
|
|
||
| interface UseHasMessagesParams { | ||
| selectedTask: { id: number } | null | ||
| selectedTaskDetail: TaskDetail | null | ||
| taskState: TaskStateData | null | ||
| streamHandlers: StreamHandlersState | ||
| } | ||
|
|
||
| /** | ||
| * Determines if there are messages to display in the chat area. | ||
| * | ||
| * Logic breakdown: | ||
| * - hasSelectedTask: Has loaded task detail with messages | ||
| * - isLoadingTask: Task is selected but details are still loading (prevents flash) | ||
| * - hasNewTaskStream: New task being created with streaming | ||
| * - hasLocalPending: Local pending message waiting to be sent | ||
| * - hasUnifiedMessages: Messages exist in state machine | ||
| */ | ||
| export function useHasMessages({ | ||
| selectedTask, | ||
| selectedTaskDetail, | ||
| taskState, | ||
| streamHandlers, | ||
| }: UseHasMessagesParams): boolean { | ||
| return useMemo(() => { | ||
| const hasSelectedTask = selectedTaskDetail?.id != null | ||
| const isLoadingTask = selectedTask != null && selectedTaskDetail == null | ||
| const hasNewTaskStream = | ||
| !selectedTaskDetail?.id && streamHandlers.pendingTaskId != null && streamHandlers.isStreaming | ||
| const hasLocalPending = streamHandlers.localPendingMessage != null | ||
| const hasUnifiedMessages = taskState?.messages != null && taskState.messages.size > 0 | ||
|
|
||
| // Fast path: task with messages loaded | ||
| if (hasSelectedTask && hasUnifiedMessages) { | ||
| return true | ||
| } | ||
|
|
||
| // Check any condition that indicates chat should be shown | ||
| return ( | ||
| hasSelectedTask || | ||
| isLoadingTask || | ||
| streamHandlers.hasPendingUserMessage || | ||
| streamHandlers.isStreaming || | ||
| hasNewTaskStream || | ||
| hasLocalPending || | ||
| hasUnifiedMessages | ||
| ) | ||
| }, [ | ||
| selectedTask, | ||
| selectedTaskDetail, | ||
| taskState?.messages, | ||
| streamHandlers.hasPendingUserMessage, | ||
| streamHandlers.isStreaming, | ||
| streamHandlers.pendingTaskId, | ||
| streamHandlers.localPendingMessage, | ||
| ]) | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |||||||||||||||||
| * Handles subscription to state changes and provides convenient accessors. | ||||||||||||||||||
| */ | ||||||||||||||||||
|
|
||||||||||||||||||
| import { useState, useEffect, useCallback, useMemo } from 'react' | ||||||||||||||||||
| import { useState, useEffect, useCallback, useMemo, useSyncExternalStore } from 'react' | ||||||||||||||||||
| import { taskStateManager, TaskStateData, UnifiedMessage, SyncOptions } from '../state' | ||||||||||||||||||
|
|
||||||||||||||||||
| export interface UseTaskStateMachineResult { | ||||||||||||||||||
|
|
@@ -36,8 +36,13 @@ export function useTaskStateMachine( | |||||||||||||||||
| ): UseTaskStateMachineResult { | ||||||||||||||||||
| const [state, setState] = useState<TaskStateData | null>(null) | ||||||||||||||||||
|
|
||||||||||||||||||
| // Check if manager is initialized | ||||||||||||||||||
| const isInitialized = taskStateManager.isInitialized() | ||||||||||||||||||
| // Reactively track manager initialization via useSyncExternalStore | ||||||||||||||||||
| const isInitialized = useSyncExternalStore( | ||||||||||||||||||
| taskStateManager.subscribeInit, | ||||||||||||||||||
| taskStateManager.getInitialized, | ||||||||||||||||||
| // SSR snapshot: always false on server | ||||||||||||||||||
|
Comment on lines
+40
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# 1. Find and read the TaskStateManager.ts file to see method definitions
echo "=== TaskStateManager.ts method definitions ==="
rg -nP --type=ts -A5 '(subscribeInit|getInitialized)' frontend/src/features/tasks/state/TaskStateManager.ts | head -50
# 2. Verify the actual callsite in useTaskStateMachine.ts
echo ""
echo "=== useTaskStateMachine.ts useSyncExternalStore usage ==="
rg -nP --type=ts -B2 -A6 'useSyncExternalStore' frontend/src/features/tasks/hooks/useTaskStateMachine.ts
# 3. Check if TaskStateManager is a class and how subscribeInit/getInitialized are defined
echo ""
echo "=== TaskStateManager class structure ==="
rg -nP --type=ts -B5 'class TaskStateManager' frontend/src/features/tasks/state/TaskStateManager.ts | head -20Repository: wecode-ai/Wegent Length of output: 1372 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check for other usages of these methods to see if this is the only problem site
echo "=== Other usages of taskStateManager.subscribeInit and getInitialized ==="
rg -nP --type=ts 'taskStateManager\.(subscribeInit|getInitialized)' frontend/src/features/tasks
echo ""
echo "=== Verify taskStateManager is an instance (singleton) ==="
rg -nP --type=ts 'export.*taskStateManager' frontend/src/features/tasks/stateRepository: wecode-ai/Wegent Length of output: 603 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify that subscribeInit actually uses 'this' context
echo "=== Full subscribeInit method body ==="
rg -nP --type=ts -A8 'subscribeInit\(listener' frontend/src/features/tasks/state/TaskStateManager.ts
echo ""
echo "=== Check if any of these methods are arrow functions ==="
rg -nP --type=ts '(subscribeInit|getInitialized)\s*=' frontend/src/features/tasks/state/TaskStateManager.ts
echo ""
echo "=== Verify this.initListeners and this.deps exist ==="
rg -nP --type=ts '(this\.initListeners|this\.deps)\s*' frontend/src/features/tasks/state/TaskStateManager.ts | head -10Repository: wecode-ai/Wegent Length of output: 417 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check the full method signatures and their dependencies on 'this'
echo "=== TaskStateManager class fields ==="
rg -nP --type=ts -B2 'class TaskStateManagerImpl' frontend/src/features/tasks/state/TaskStateManager.ts | head -30
echo ""
echo "=== getInitialized full method ==="
rg -nP --type=ts -A3 'getInitialized\(\)' frontend/src/features/tasks/state/TaskStateManager.ts
echo ""
echo "=== Check where initListeners and deps are initialized ==="
rg -nP --type=ts '(this\.initListeners|this\.deps)\s*=' frontend/src/features/tasks/state/TaskStateManager.ts | head -5Repository: wecode-ai/Wegent Length of output: 411 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Let's verify how useSyncExternalStore calls these functions to understand the binding issue
echo "=== React useSyncExternalStore API check ==="
echo "useSyncExternalStore signature: subscribe(listener), getSnapshot(), getServerSnapshot()"
echo ""
echo "The issue is that when you pass taskStateManager.subscribeInit directly,"
echo "React will call it as: subscribeInit(listener) without 'this' context"
echo ""
echo "Let's verify the method needs 'this' by checking all 'this' usages:"
rg -nP --type=ts 'this\.(initListeners|deps)' frontend/src/features/tasks/state/TaskStateManager.tsRepository: wecode-ai/Wegent Length of output: 726 Bind Lines 41–42 pass unbound instance methods. Both Proposed fix const isInitialized = useSyncExternalStore(
- taskStateManager.subscribeInit,
- taskStateManager.getInitialized,
+ listener => taskStateManager.subscribeInit(listener),
+ () => taskStateManager.getInitialized(),
// SSR snapshot: always false on server
() => false
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| () => false | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| // Subscribe to state changes | ||||||||||||||||||
| // Subscribe to state changes | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 468
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 87
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 203
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 8059
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 258
🏁 Script executed:
Repository: wecode-ai/Wegent
Length of output: 276
Prevent
get()callers from overriding the HTTP method.At line 124,
{ method: 'GET', ...options }allowsoptions.methodto silently change this into a non-GET request. While no current callsite exploits this, it breaks the semantic contract of the method.🔧 Proposed fix
🤖 Prompt for AI Agents