Merge remote-tracking branch 'origin/feat/support-agent-sandbox' into pre-align-hitl-frontend

web/app/components/workflow/hooks/use-inspect-vars-crud-common.ts

@@ -224,7 +224,7 @@ export const useInspectVarsCrudCommon = ({
     await Promise.all([
       invalidateConversationVarValues(),
       invalidateSysVarValues(),
-      invalidateSandboxFiles(),
+      invalidateSandboxFiles({ refetchDownloadFile: false }),
     ])
     deleteAllInspectVars()
     handleEdgeCancelRunningStatus()

web/app/components/workflow/variable-inspect/artifacts-tab.spec.tsx

@@ -0,0 +1,141 @@
+import type { SandboxFileNode, SandboxFileTreeNode } from '@/types/sandbox-file'
+import { fireEvent, render, screen, waitFor } from '@testing-library/react'
+import ArtifactsTab from './artifacts-tab'
+import { InspectTab } from './types'
+
+type MockStoreState = {
+  appId: string | undefined
+  workflowRunningData?: {
+    result?: {
+      status?: string
+    }
+  }
+  isResponding: boolean
+  bottomPanelWidth: number
+}
+
+const mocks = vi.hoisted(() => ({
+  storeState: {
+    appId: 'app-1',
+    workflowRunningData: undefined,
+    isResponding: false,
+    bottomPanelWidth: 640,
+  } as MockStoreState,
+  treeData: undefined as SandboxFileTreeNode[] | undefined,
+  flatData: [] as SandboxFileNode[],
+  hasFiles: false,
+  isLoading: false,
+  fetchDownloadUrl: vi.fn(),
+  useSandboxFileDownloadUrl: vi.fn(),
+}))
+
+vi.mock('../store', () => ({
+  useStore: (selector: (state: MockStoreState) => unknown) => selector(mocks.storeState),
+}))
+
+vi.mock('@/service/use-sandbox-file', () => ({
+  useSandboxFilesTree: () => ({
+    data: mocks.treeData,
+    flatData: mocks.flatData,
+    hasFiles: mocks.hasFiles,
+    isLoading: mocks.isLoading,
+  }),
+  useDownloadSandboxFile: () => ({
+    mutateAsync: mocks.fetchDownloadUrl,
+    isPending: false,
+  }),
+  useSandboxFileDownloadUrl: (...args: unknown[]) => mocks.useSandboxFileDownloadUrl(...args),
+}))
+
+vi.mock('@/context/i18n', () => ({
+  useDocLink: () => (path: string) => path,
+}))
+
+vi.mock('@/app/components/base/features/hooks', () => ({
+  useFeatures: (selector: (state: { features: { sandbox: { enabled: boolean } } }) => unknown) => selector({
+    features: {
+      sandbox: {
+        enabled: true,
+      },
+    },
+  }),
+}))
+
+vi.mock('@/utils/download', () => ({
+  downloadUrl: vi.fn(),
+}))
+
+const createTreeFileNode = (overrides: Partial<SandboxFileTreeNode> = {}): SandboxFileTreeNode => ({
+  id: 'a.txt',
+  name: 'a.txt',
+  path: 'a.txt',
+  node_type: 'file',
+  size: 128,
+  mtime: 1700000000,
+  extension: 'txt',
+  children: [],
+  ...overrides,
+})
+
+const createFlatFileNode = (overrides: Partial<SandboxFileNode> = {}): SandboxFileNode => ({
+  path: 'a.txt',
+  is_dir: false,
+  size: 128,
+  mtime: 1700000000,
+  extension: 'txt',
+  ...overrides,
+})
+
+describe('ArtifactsTab', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mocks.storeState.appId = 'app-1'
+    mocks.storeState.workflowRunningData = undefined
+    mocks.storeState.isResponding = false
+    mocks.storeState.bottomPanelWidth = 640
+
+    mocks.treeData = [createTreeFileNode()]
+    mocks.flatData = [createFlatFileNode()]
+    mocks.hasFiles = true
+    mocks.isLoading = false
+    mocks.useSandboxFileDownloadUrl.mockReturnValue({
+      data: undefined,
+      isLoading: false,
+    })
+  })
+
+  it('should stop using stale file path for download url query after files are cleared', async () => {
+    const headerProps = {
+      activeTab: InspectTab.Artifacts,
+      onTabChange: vi.fn(),
+      onClose: vi.fn(),
+    }
+
+    const { rerender } = render(<ArtifactsTab {...headerProps} />)
+
+    fireEvent.click(screen.getByRole('button', { name: 'a.txt' }))
+
+    await waitFor(() => {
+      expect(mocks.useSandboxFileDownloadUrl).toHaveBeenCalledWith(
+        'app-1',
+        'a.txt',
+        { retry: false },
+      )
+    })
+
+    mocks.treeData = undefined
+    mocks.flatData = []
+    mocks.hasFiles = false
+
+    rerender(<ArtifactsTab {...headerProps} />)
+
+    await waitFor(() => {
+      const lastCall = mocks.useSandboxFileDownloadUrl.mock.calls.at(-1)
+      expect(lastCall).toEqual([
+        'app-1',
+        undefined,
+        { retry: false },
+      ])
+    })
+  })
+})

web/app/components/workflow/variable-inspect/artifacts-tab.tsx

@@ -4,7 +4,7 @@ import {
   RiCloseLine,
   RiMenuLine,
 } from '@remixicon/react'
-import { useCallback, useState } from 'react'
+import { useCallback, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import ActionButton from '@/app/components/base/action-button'
 import SearchLinesSparkle from '@/app/components/base/icons/src/vender/knowledge/SearchLinesSparkle'
@@ -62,15 +62,27 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => {
   )
   const isResponding = useStore(s => s.isResponding)
 
-  const { data: treeData, hasFiles, isLoading } = useSandboxFilesTree(appId, {
+  const { data: treeData, flatData, hasFiles, isLoading } = useSandboxFilesTree(appId, {
     enabled: !!appId,
     refetchInterval: (isWorkflowRunning || isResponding) ? 5000 : false,
   })
   const { mutateAsync: fetchDownloadUrl, isPending: isDownloading } = useDownloadSandboxFile(appId)
   const [selectedFile, setSelectedFile] = useState<SandboxFileTreeNode | null>(null)
+  const selectedFilePath = useMemo(() => {
+    if (!selectedFile)
+      return undefined
+
+    const selectedExists = flatData?.some(
+      node => !node.is_dir && node.path === selectedFile.path,
+    ) ?? false
+
+    return selectedExists ? selectedFile.path : undefined
+  }, [flatData, selectedFile])
+
   const { data: downloadUrlData, isLoading: isDownloadUrlLoading } = useSandboxFileDownloadUrl(
     appId,
-    selectedFile?.path,
+    selectedFilePath,
+    { retry: false },
   )
 
   const handleFileSelect = useCallback((node: SandboxFileTreeNode) => {
@@ -113,7 +125,7 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => {
     )
   }
 
-  const file = selectedFile
+  const file = selectedFilePath ? selectedFile : null
   const parts = file?.path.split('/') ?? []
   let cumPath = ''
   const pathSegments = parts.map((part, i) => {
@@ -130,7 +142,7 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => {
             data={treeData}
             onDownload={handleTreeDownload}
             onSelect={handleFileSelect}
-            selectedPath={selectedFile?.path}
+            selectedPath={selectedFilePath}
             isDownloading={isDownloading}
           />
         </div>

web/service/use-sandbox-file.spec.tsx

@@ -0,0 +1,60 @@
+import type { ReactNode } from 'react'
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
+import { act, renderHook } from '@testing-library/react'
+import { consoleQuery } from './client'
+import { useInvalidateSandboxFiles } from './use-sandbox-file'
+
+const createQueryClient = () => new QueryClient({
+  defaultOptions: {
+    queries: {
+      retry: false,
+    },
+  },
+})
+
+const createWrapper = (queryClient: QueryClient) => {
+  return ({ children }: { children: ReactNode }) => (
+    <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+  )
+}
+
+describe('useInvalidateSandboxFiles', () => {
+  it('should keep download query refetch enabled by default', async () => {
+    const queryClient = createQueryClient()
+    const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries').mockResolvedValue(undefined)
+    const { result } = renderHook(() => useInvalidateSandboxFiles(), {
+      wrapper: createWrapper(queryClient),
+    })
+
+    await act(async () => {
+      await result.current()
+    })
+
+    expect(invalidateSpy).toHaveBeenNthCalledWith(1, {
+      queryKey: consoleQuery.sandboxFile.listFiles.key(),
+    })
+    expect(invalidateSpy).toHaveBeenNthCalledWith(2, {
+      queryKey: consoleQuery.sandboxFile.downloadFile.key(),
+    })
+  })
+
+  it('should disable download query refetch when requested', async () => {
+    const queryClient = createQueryClient()
+    const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries').mockResolvedValue(undefined)
+    const { result } = renderHook(() => useInvalidateSandboxFiles(), {
+      wrapper: createWrapper(queryClient),
+    })
+
+    await act(async () => {
+      await result.current({ refetchDownloadFile: false })
+    })
+
+    expect(invalidateSpy).toHaveBeenNthCalledWith(1, {
+      queryKey: consoleQuery.sandboxFile.listFiles.key(),
+    })
+    expect(invalidateSpy).toHaveBeenNthCalledWith(2, {
+      queryKey: consoleQuery.sandboxFile.downloadFile.key(),
+      refetchType: 'none',
+    })
+  })
+})

web/service/use-sandbox-file.ts

@@ -14,6 +14,15 @@ type UseGetSandboxFilesOptions = {
   refetchInterval?: number | false
 }
 
+type UseSandboxFileDownloadUrlOptions = {
+  enabled?: boolean
+  retry?: boolean | number
+}
+
+type InvalidateSandboxFilesOptions = {
+  refetchDownloadFile?: boolean
+}
+
 export function useGetSandboxFiles(
   appId: string | undefined,
   options?: UseGetSandboxFilesOptions,
@@ -39,6 +48,7 @@ export function useGetSandboxFiles(
 export function useSandboxFileDownloadUrl(
   appId: string | undefined,
   path: string | undefined,
+  options?: UseSandboxFileDownloadUrlOptions,
 ) {
   return useQuery({
     queryKey: consoleQuery.sandboxFile.downloadFile.queryKey({
@@ -48,19 +58,22 @@ export function useSandboxFileDownloadUrl(
       params: { appId: appId! },
       body: { path: path! },
     }),
-    enabled: !!appId && !!path,
+    enabled: !!appId && !!path && (options?.enabled ?? true),
+    retry: options?.retry,
   })
 }
 
 export function useInvalidateSandboxFiles() {
   const queryClient = useQueryClient()
-  return useCallback(() => {
+  return useCallback((options?: InvalidateSandboxFilesOptions) => {
+    const shouldRefetchDownloadFile = options?.refetchDownloadFile ?? true
     return Promise.all([
       queryClient.invalidateQueries({
         queryKey: consoleQuery.sandboxFile.listFiles.key(),
       }),
       queryClient.invalidateQueries({
         queryKey: consoleQuery.sandboxFile.downloadFile.key(),
+        ...(shouldRefetchDownloadFile ? {} : { refetchType: 'none' as const }),
       }),
     ])
   }, [queryClient])