-
Notifications
You must be signed in to change notification settings - Fork 93
fix(frontend): show 'device deleted' status for tasks with deleted de… #857
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
3c7b349
394fdcc
63a496c
a8da2ea
36f85d6
80df098
99fdd95
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 |
|---|---|---|
|
|
@@ -48,16 +48,29 @@ export function ChatPageMobile() { | |
| const { selectedDeviceId, devices } = useDevices() | ||
| const selectedDevice = devices.find(d => d.device_id === selectedDeviceId) | ||
|
|
||
| // For existing tasks, also check the task's device_id | ||
| const taskDevice = selectedTaskDetail?.device_id | ||
| ? devices.find(d => d.device_id === selectedTaskDetail.device_id) | ||
| : null | ||
| const isTaskDeviceDeleted = | ||
| selectedTaskDetail?.device_id && | ||
| !devices.some(d => d.device_id === selectedTaskDetail.device_id) | ||
| const isTaskDeviceOffline = taskDevice?.status === 'offline' | ||
|
Comment on lines
+51
to
+58
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. Race condition: false positive during initial device loading. Same issue as 🐛 Proposed fix: Check loading state before determining deletion status // Device context - when a device is selected, switch to 'task' mode
- const { selectedDeviceId, devices } = useDevices()
+ const { selectedDeviceId, devices, isLoading: isDevicesLoading } = useDevices()
const selectedDevice = devices.find(d => d.device_id === selectedDeviceId)
// For existing tasks, also check the task's device_id
const taskDevice = selectedTaskDetail?.device_id
? devices.find(d => d.device_id === selectedTaskDetail.device_id)
: null
const isTaskDeviceDeleted =
+ !isDevicesLoading &&
selectedTaskDetail?.device_id &&
!devices.some(d => d.device_id === selectedTaskDetail.device_id)
const isTaskDeviceOffline = taskDevice?.status === 'offline'🤖 Prompt for AI Agents |
||
|
|
||
| // Determine taskType based on device selection | ||
| // When a device is selected, use 'task' mode (same as /devices/chat) | ||
| // Otherwise, use 'chat' mode | ||
| const taskType = selectedDeviceId ? 'task' : 'chat' | ||
|
|
||
| // Compute disabled reason for device mode | ||
| const disabledReason = | ||
| selectedDeviceId && (!selectedDevice || selectedDevice.status === 'offline') | ||
| // Consider both currently selected device and task's associated device | ||
| const disabledReason = isTaskDeviceDeleted | ||
| ? t('devices:device_deleted_hint') | ||
| : isTaskDeviceOffline | ||
| ? t('devices:device_offline_cannot_send') | ||
| : undefined | ||
| : selectedDeviceId && (!selectedDevice || selectedDevice.status === 'offline') | ||
| ? t('devices:device_offline_cannot_send') | ||
| : undefined | ||
|
|
||
| // Get current task title for top navigation | ||
| const currentTaskTitle = selectedTaskDetail?.title | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
|
|
||
| 'use client' | ||
|
|
||
| import { useEffect, useState, useCallback } from 'react' | ||
| import { useEffect, useState, useCallback, useMemo } from 'react' | ||
| import { useRouter, useSearchParams } from 'next/navigation' | ||
| import TopNavigation from '@/features/layout/TopNavigation' | ||
| import { | ||
|
|
@@ -123,9 +123,22 @@ export default function DeviceChatPage() { | |
| // Get selected device info | ||
| const selectedDevice = devices.find(d => d.device_id === selectedDeviceId) | ||
|
|
||
| // For existing tasks, use task's device_id instead of selectedDeviceId for display | ||
| const taskDevice = selectedTaskDetail?.device_id | ||
| ? devices.find(d => d.device_id === selectedTaskDetail.device_id) | ||
| : null | ||
| // Task is considered to have messages if it has a non-empty title and was created | ||
| const hasMessages = !!(selectedTaskDetail && selectedTaskDetail.id) | ||
|
|
||
| // Check if selected device is OpenClaw type | ||
| const isOpenClaw = selectedDevice ? isOpenClawDevice(selectedDevice) : false | ||
|
|
||
| // Check if task was associated with a device that has been deleted | ||
| const isTaskDeviceDeleted = useMemo(() => { | ||
| if (!selectedTaskDetail?.device_id) return false | ||
| return !devices.some(d => d.device_id === selectedTaskDetail.device_id) | ||
| }, [selectedTaskDetail?.device_id, devices]) | ||
|
Comment on lines
+136
to
+140
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. Race condition: false positive during initial device loading. The 🐛 Proposed fix: Check `isLoading` before determining deletion status+ const { devices, selectedDeviceId, setSelectedDeviceId, isLoading: isDevicesLoading } = useDevices()
// Check if task was associated with a device that has been deleted
const isTaskDeviceDeleted = useMemo(() => {
+ // Don't report as deleted while devices are still loading
+ if (isDevicesLoading) return false
if (!selectedTaskDetail?.device_id) return false
return !devices.some(d => d.device_id === selectedTaskDetail.device_id)
- }, [selectedTaskDetail?.device_id, devices])
+ }, [selectedTaskDetail?.device_id, devices, isDevicesLoading])🤖 Prompt for AI Agents |
||
|
|
||
| return ( | ||
| <div className="flex smart-h-screen bg-base text-text-primary box-border"> | ||
| {/* URL parameter sync */} | ||
|
|
@@ -162,30 +175,57 @@ export default function DeviceChatPage() { | |
| onMembersChanged={handleMembersChanged} | ||
| isSidebarCollapsed={isCollapsed} | ||
| > | ||
| {/* Device selector in top bar */} | ||
| <div className="flex items-center gap-2 mr-2"> | ||
| <Monitor className="w-4 h-4 text-text-muted" /> | ||
| <select | ||
| value={selectedDeviceId || ''} | ||
| onChange={e => handleDeviceSelect(e.target.value)} | ||
| className="bg-surface border border-border rounded-md px-2 py-1 text-sm focus:outline-none focus:ring-2 focus:ring-primary/20" | ||
| > | ||
| <option value="" disabled> | ||
| {t('select_device')} | ||
| </option> | ||
| {devices.map(device => ( | ||
| <option key={device.device_id} value={device.device_id}> | ||
| {device.name} ( | ||
| {device.status === 'online' | ||
| ? t('status_online') | ||
| : device.status === 'busy' | ||
| ? t('status_busy') | ||
| : t('status_offline')} | ||
| ) | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| {/* Device selector in top bar - hide when task's device is deleted */} | ||
| {!isTaskDeviceDeleted && ( | ||
| <div className="flex items-center gap-2 mr-2"> | ||
| <Monitor className="w-4 h-4 text-text-muted" /> | ||
| {/* For existing tasks with messages, show read-only device info */} | ||
| {hasMessages && taskDevice ? ( | ||
| <div className="flex items-center gap-1.5 bg-surface border border-border rounded-md px-2 py-1 text-sm"> | ||
| <span className="truncate max-w-[150px]">{taskDevice.name}</span> | ||
| <span | ||
| className={`w-2 h-2 rounded-full ${ | ||
| taskDevice.status === 'online' | ||
| ? 'bg-green-500' | ||
| : taskDevice.status === 'busy' | ||
| ? 'bg-yellow-500' | ||
| : 'bg-gray-400' | ||
| }`} | ||
| /> | ||
| <span className="text-text-muted text-xs"> | ||
| ( | ||
| {taskDevice.status === 'online' | ||
| ? t('status_online') | ||
| : taskDevice.status === 'busy' | ||
| ? t('status_busy') | ||
| : t('status_offline')} | ||
| ) | ||
| </span> | ||
| </div> | ||
| ) : ( | ||
| <select | ||
| value={selectedDeviceId || ''} | ||
| onChange={e => handleDeviceSelect(e.target.value)} | ||
| className="bg-surface border border-border rounded-md px-2 py-1 text-sm focus:outline-none focus:ring-2 focus:ring-primary/20" | ||
| > | ||
| <option value="" disabled> | ||
| {t('select_device')} | ||
| </option> | ||
| {devices.map(device => ( | ||
| <option key={device.device_id} value={device.device_id}> | ||
| {device.name} ( | ||
| {device.status === 'online' | ||
| ? t('status_online') | ||
| : device.status === 'busy' | ||
| ? t('status_busy') | ||
| : t('status_offline')} | ||
| ) | ||
| </option> | ||
| ))} | ||
| </select> | ||
| )} | ||
| </div> | ||
| )} | ||
| {isMobile ? <ThemeToggle /> : <GithubStarButton />} | ||
| </TopNavigation> | ||
|
|
||
|
|
@@ -199,9 +239,15 @@ export default function DeviceChatPage() { | |
| taskType="task" | ||
| onRefreshTeams={handleRefreshTeams} | ||
| disabledReason={ | ||
| !selectedDevice || selectedDevice.status === 'offline' | ||
| ? t('device_offline_cannot_send') | ||
| : undefined | ||
| isTaskDeviceDeleted | ||
| ? t('device_deleted_hint') | ||
| : hasMessages && taskDevice?.status === 'offline' | ||
| ? t('device_offline_cannot_send') | ||
| : hasMessages && !selectedTaskDetail?.device_id | ||
| ? undefined // Task was created with cloud mode, allow sending | ||
| : !selectedDevice || selectedDevice.status === 'offline' | ||
| ? t('device_offline_cannot_send') | ||
| : undefined | ||
| } | ||
| hideSelectors={isOpenClaw} | ||
| /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,7 @@ import { useAttachmentUpload } from '../hooks/useAttachmentUpload' | |||||||||||||||||||||||||||||||||||||
| import { useSchemeMessageActions } from '@/lib/scheme' | ||||||||||||||||||||||||||||||||||||||
| import { useSkillSelector } from '../../hooks/useSkillSelector' | ||||||||||||||||||||||||||||||||||||||
| import { useModelSelection } from '../../hooks/useModelSelection' | ||||||||||||||||||||||||||||||||||||||
| import { useDevices } from '@/contexts/DeviceContext' | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Threshold in pixels for determining when to collapse selectors. | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -112,6 +113,15 @@ function ChatAreaContent({ | |||||||||||||||||||||||||||||||||||||
| // Task context | ||||||||||||||||||||||||||||||||||||||
| const { selectedTaskDetail, setSelectedTask, accessDenied } = useTaskContext() | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Device context - for detecting deleted device | ||||||||||||||||||||||||||||||||||||||
| const { devices } = useDevices() | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Check if task was associated with a device that has been deleted | ||||||||||||||||||||||||||||||||||||||
| const isTaskDeviceDeleted = useMemo(() => { | ||||||||||||||||||||||||||||||||||||||
| if (!selectedTaskDetail?.device_id) return false | ||||||||||||||||||||||||||||||||||||||
| return !devices.some(d => d.device_id === selectedTaskDetail.device_id) | ||||||||||||||||||||||||||||||||||||||
| }, [selectedTaskDetail?.device_id, devices]) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+116
to
+123
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. Race condition: false positive during initial device loading. Same issue as in the page components: when 🐛 Proposed fix: Guard against loading state // Device context - for detecting deleted device
- const { devices } = useDevices()
+ const { devices, isLoading: isDevicesLoading } = useDevices()
// Check if task was associated with a device that has been deleted
const isTaskDeviceDeleted = useMemo(() => {
+ // Don't report as deleted while devices are still loading
+ if (isDevicesLoading) return false
if (!selectedTaskDetail?.device_id) return false
return !devices.some(d => d.device_id === selectedTaskDetail.device_id)
- }, [selectedTaskDetail?.device_id, devices])
+ }, [selectedTaskDetail?.device_id, devices, isDevicesLoading])📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Use useTaskStateMachine hook for reactive state updates (SINGLE SOURCE OF TRUTH per AGENTS.md) | ||||||||||||||||||||||||||||||||||||||
| const { state: taskState } = useTaskStateMachine(selectedTaskDetail?.id) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
@@ -1040,6 +1050,8 @@ function ChatAreaContent({ | |||||||||||||||||||||||||||||||||||||
| onGenerateModeChange, | ||||||||||||||||||||||||||||||||||||||
| // Hide all selectors (for OpenClaw devices) | ||||||||||||||||||||||||||||||||||||||
| hideSelectors, | ||||||||||||||||||||||||||||||||||||||
| // Whether the task's device has been deleted | ||||||||||||||||||||||||||||||||||||||
| isTaskDeviceDeleted, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -327,6 +327,13 @@ export function DeviceSelectorTab({ | |
| : null | ||
| }, [devices, selectedDeviceId, hasMessages, taskDeviceId]) | ||
|
|
||
| // Check if the task was associated with a device that has been deleted | ||
| const isTaskDeviceDeleted = useMemo(() => { | ||
| if (!hasMessages || !taskDeviceId) return false | ||
| // If taskDeviceId exists but device not found in devices list, it means the device was deleted | ||
| return !devices.some(device => device.device_id === taskDeviceId) | ||
| }, [hasMessages, taskDeviceId, devices]) | ||
|
Comment on lines
+330
to
+335
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. Same race condition as in This memo has the same issue as the one in 🐛 Proposed fix: Guard against loading state- const { devices, selectedDeviceId, setSelectedDeviceId, isLoading } = useDevices()
+ const { devices, selectedDeviceId, setSelectedDeviceId, isLoading: isDevicesLoading } = useDevices()
// Check if the task was associated with a device that has been deleted
const isTaskDeviceDeleted = useMemo(() => {
+ // Don't report as deleted while devices are still loading
+ if (isDevicesLoading) return false
if (!hasMessages || !taskDeviceId) return false
// If taskDeviceId exists but device not found in devices list, it means the device was deleted
return !devices.some(device => device.device_id === taskDeviceId)
- }, [hasMessages, taskDeviceId, devices])
+ }, [hasMessages, taskDeviceId, devices, isDevicesLoading])Note: const isTaskDeviceDeleted = useMemo(() => {
+ if (isLoading) return false
if (!hasMessages || !taskDeviceId) return false
return !devices.some(device => device.device_id === taskDeviceId)
- }, [hasMessages, taskDeviceId, devices])
+ }, [hasMessages, taskDeviceId, devices, isLoading])🤖 Prompt for AI Agents |
||
|
|
||
| useEffect(() => { | ||
| if (hasMessages || isLoading || autoSelectionInitializedRef.current) { | ||
| return | ||
|
|
@@ -450,6 +457,40 @@ export function DeviceSelectorTab({ | |
|
|
||
| // Read-only mode for existing chats | ||
| if (hasMessages) { | ||
| // When device is deleted, don't show any selector - the prompt will be shown in the input area | ||
| if (isTaskDeviceDeleted) { | ||
| return null | ||
| } | ||
|
|
||
| // If task has device_id but device not found (and not deleted), don't show "Public Mode" | ||
| // This handles the case where devices list is still loading or data inconsistency | ||
| if (taskDeviceId && !selectedDevice) { | ||
| // Show a loading/unknown state instead of misleading "Public Mode" | ||
| return ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div | ||
| className={cn( | ||
| 'flex items-center gap-1.5 px-2 py-1.5 rounded-md', | ||
| 'bg-surface border border-border', | ||
| 'text-xs text-text-secondary', | ||
| className | ||
| )} | ||
| > | ||
| <Monitor className="w-3.5 h-3.5" /> | ||
| <span className="truncate max-w-[160px]">{t('local_device_prefix')}...</span> | ||
| <span className="w-1.5 h-1.5 rounded-full bg-gray-400" /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="top"> | ||
| <p>{t('device_offline_hint')}</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
|
|
||
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.
Race condition: false positive during initial device loading.
The
isTaskDeviceDeletedcheck doesn't account for the devices loading state. Whendevicesinitializes as an empty array while fetching, any task with adevice_idwill incorrectly show "Device Deleted" status until the fetch completes.This is the same issue identified in
frontend/src/app/(tasks)/devices/chat/page.tsx. Consider extractingisLoadingfromuseDevices()and guarding against it.🐛 Proposed fix: Check loading state before determining deletion status
🤖 Prompt for AI Agents