diff --git a/.gitignore b/.gitignore index e47c543..96c7a64 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ PRPs/stories/ tmp/ temp/ UAT/ + +.DS_Store diff --git a/archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts b/archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts index 1d51ff5..874499e 100644 --- a/archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts +++ b/archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts @@ -4,7 +4,7 @@ */ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { useEffect, useMemo, useState } from "react"; +import { useMemo, useState } from "react"; import { createOptimisticEntity, createOptimisticId } from "@/features/shared/optimistic"; import { useActiveOperations } from "../../progress/hooks"; import { progressKeys } from "../../progress/hooks/useProgressQueries"; @@ -65,7 +65,6 @@ export function useKnowledgeItemChunks( sourceId: string | null, opts?: { domain?: string; limit?: number; offset?: number }, ) { - // TODO: Phase 4 - Add explicit typing: useQuery or appropriate return type // See PRPs/local/frontend-state-management-refactor.md Phase 4: Configure Request Deduplication return useQuery({ queryKey: sourceId ? knowledgeKeys.chunks(sourceId, opts) : DISABLED_QUERY_KEY, @@ -116,21 +115,29 @@ export function useCrawlUrl() { >({ mutationFn: (request: CrawlRequest) => knowledgeService.crawlUrl(request), onMutate: async (request) => { - // TODO: Phase 3 - Fix optimistic updates writing to wrong cache - // knowledgeKeys.lists() is never queried - actual data comes from knowledgeKeys.summaries(filter) - // This makes all optimistic updates invisible. Should either: - // 1. Remove optimistic updates for knowledge items - // 2. Update all summary caches with optimistic data - // 3. Create a real query that uses lists() - // See: PRPs/local/frontend-state-management-refactor.md Phase 3 - // Cancel any outgoing refetches to prevent race conditions - await queryClient.cancelQueries({ queryKey: knowledgeKeys.lists() }); await queryClient.cancelQueries({ queryKey: knowledgeKeys.summariesPrefix() }); await queryClient.cancelQueries({ queryKey: progressKeys.active() }); + // TODO: Fix invisible optimistic updates + // ISSUE: Optimistic updates are applied to knowledgeKeys.summaries(filter) queries, + // but the UI component (KnowledgeView) queries with dynamic filters that we don't have access to here. + // This means optimistic updates only work if the filter happens to match what's being viewed. + // + // CURRENT BEHAVIOR: + // - We update all cached summaries queries (lines 158-179 below) + // - BUT if the user changes filters after mutation starts, they won't see the optimistic update + // - AND we have no way to know what filter the user is currently viewing + // + // PROPER FIX requires one of: + // 1. Pass current filter from KnowledgeView to mutation hooks (prop drilling) + // 2. Create KnowledgeFilterContext to share filter state + // 3. Restructure to have a single source of truth query key like other features + // + // IMPACT: Users don't see immediate feedback when adding knowledge items - items only + // appear after the server responds (usually 1-3 seconds later) + // Snapshot the previous values for rollback - const previousKnowledge = queryClient.getQueryData(knowledgeKeys.lists()); const previousSummaries = queryClient.getQueriesData({ queryKey: knowledgeKeys.summariesPrefix(), }); @@ -165,14 +172,7 @@ export function useCrawlUrl() { } as Omit); const tempItemId = optimisticItem.id; - // Add optimistic knowledge item to the list - queryClient.setQueryData(knowledgeKeys.lists(), (old) => { - if (!old) return [optimisticItem]; - // Add at the beginning for visibility - return [optimisticItem, ...old]; - }); - - // Respect each cache's filter (knowledge_type, tags, etc.) + // Update all summaries caches with optimistic data, respecting each cache's filter const entries = queryClient.getQueriesData({ queryKey: knowledgeKeys.summariesPrefix(), }); @@ -229,28 +229,12 @@ export function useCrawlUrl() { }); // Return context for rollback and replacement - return { previousKnowledge, previousSummaries, previousOperations, tempProgressId, tempItemId }; + return { previousSummaries, previousOperations, tempProgressId, tempItemId }; }, onSuccess: (response, _variables, context) => { // Replace temporary IDs with real ones from the server if (context) { - // Update knowledge item with real source_id if we get it - queryClient.setQueryData(knowledgeKeys.lists(), (old) => { - if (!old) return old; - return old.map((item) => { - if (item.id === context.tempItemId) { - // Update with real progress ID, but keep the optimistic item - // The real item will come through polling/invalidation - return { - ...item, - source_id: response.progressId, - }; - } - return item; - }); - }); - - // Also update summaries cache with real progress ID + // Update summaries cache with real progress ID queryClient.setQueriesData({ queryKey: knowledgeKeys.summariesPrefix() }, (old) => { if (!old) return old; return { @@ -289,7 +273,6 @@ export function useCrawlUrl() { } // Invalidate to get fresh data - queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }); queryClient.invalidateQueries({ queryKey: progressKeys.active() }); showToast(`Crawl started: ${response.message}`, "success"); @@ -299,9 +282,6 @@ export function useCrawlUrl() { }, onError: (error, _variables, context) => { // Rollback optimistic updates on error - if (context?.previousKnowledge) { - queryClient.setQueryData(knowledgeKeys.lists(), context.previousKnowledge); - } if (context?.previousSummaries) { // Rollback all summary queries for (const [queryKey, data] of context.previousSummaries) { @@ -473,12 +453,9 @@ export function useUploadDocument() { }); } - // Invalidate queries to get fresh data - with a short delay for fast uploads - setTimeout(() => { - queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }); - queryClient.invalidateQueries({ queryKey: knowledgeKeys.summariesPrefix() }); - queryClient.invalidateQueries({ queryKey: progressKeys.active() }); - }, 1000); + // Only invalidate progress to start tracking the new operation + // The lists/summaries will refresh automatically via polling when operations are active + queryClient.invalidateQueries({ queryKey: progressKeys.active() }); // Don't show success here - upload is just starting in background // Success/failure will be shown via progress polling @@ -514,7 +491,6 @@ export function useStopCrawl() { }, onError: (error, progressId) => { // If it's a 404, the operation might have already completed or been cancelled - // TODO: Phase 4 - Improve error type safety, create proper error interface instead of 'as any' // See PRPs/local/frontend-state-management-refactor.md Phase 4: Configure Request Deduplication const is404Error = (error as any)?.statusCode === 404 || @@ -705,8 +681,7 @@ export function useUpdateKnowledgeItem() { // Invalidate all related queries queryClient.invalidateQueries({ queryKey: knowledgeKeys.detail(sourceId) }); - queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }); - queryClient.invalidateQueries({ queryKey: knowledgeKeys.summariesPrefix() }); // Add summaries cache + queryClient.invalidateQueries({ queryKey: knowledgeKeys.summariesPrefix() }); }, }); } @@ -726,10 +701,8 @@ export function useRefreshKnowledgeItem() { // Remove the item from cache as it's being refreshed queryClient.removeQueries({ queryKey: knowledgeKeys.detail(sourceId) }); - // Invalidate list after a delay - setTimeout(() => { - queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }); - }, 5000); + // Invalidate summaries immediately - backend is consistent after refresh initiation + queryClient.invalidateQueries({ queryKey: knowledgeKeys.summariesPrefix() }); return data; }, @@ -746,8 +719,6 @@ export function useRefreshKnowledgeItem() { * Only polls when there are active operations that we started */ export function useKnowledgeSummaries(filter?: KnowledgeItemsFilter) { - const queryClient = useQueryClient(); - // Track active crawl IDs locally - only set when we start a crawl/refresh const [activeCrawlIds, setActiveCrawlIds] = useState([]); @@ -783,37 +754,8 @@ export function useKnowledgeSummaries(filter?: KnowledgeItemsFilter) { }); // When operations complete, remove them from tracking - useEffect(() => { - const completedOps = activeOperations.filter( - (op) => op.status === "completed" || op.status === "failed" || op.status === "error", - ); - - if (completedOps.length > 0) { - // Remove completed operations from tracking - setActiveCrawlIds((prev) => prev.filter((id) => !completedOps.some((op) => op.progressId === id))); - - // Check if any completed operations are uploads (they complete faster) - const hasCompletedUpload = completedOps.some((op) => op.operation_type === "upload" || op.type === "upload"); - - // Use shorter delay for uploads (1s) vs crawls (5s) to handle fast operations - const delay = hasCompletedUpload ? 1000 : 5000; - - // Invalidate after a delay to allow backend database to become consistent - const timer = setTimeout(() => { - // Invalidate all summaries regardless of filter - queryClient.invalidateQueries({ queryKey: knowledgeKeys.summariesPrefix() }); - // Also invalidate lists for consistency - queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }); - - // For uploads, also refetch immediately to ensure UI shows the item - if (hasCompletedUpload) { - queryClient.refetchQueries({ queryKey: knowledgeKeys.summariesPrefix() }); - } - }, delay); - - return () => clearTimeout(timer); - } - }, [activeOperations, queryClient]); + // Trust smart polling to handle eventual consistency - no manual invalidation needed + // Active operations are already tracked and polling handles updates when operations complete return { ...summaryQuery, diff --git a/archon-ui-main/src/features/mcp/components/McpConfigSection.tsx b/archon-ui-main/src/features/mcp/components/McpConfigSection.tsx index c98821e..3a9a11f 100644 --- a/archon-ui-main/src/features/mcp/components/McpConfigSection.tsx +++ b/archon-ui-main/src/features/mcp/components/McpConfigSection.tsx @@ -189,7 +189,7 @@ export const McpConfigSection: React.FC = ({ config, stat const handleCopyConfig = async () => { const configText = ideConfigurations[selectedIDE].configGenerator(config); const result = await copyToClipboard(configText); - + if (result.success) { showToast("Configuration copied to clipboard", "success"); } else { @@ -212,7 +212,7 @@ export const McpConfigSection: React.FC = ({ config, stat const handleClaudeCodeCommand = async () => { const command = `claude mcp add --transport http archon http://${config.host}:${config.port}/mcp`; const result = await copyToClipboard(command); - + if (result.success) { showToast("Command copied to clipboard", "success"); } else { diff --git a/archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts b/archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts index e189e86..b39cbb1 100644 --- a/archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts +++ b/archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts @@ -1,5 +1,10 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { createOptimisticEntity, replaceOptimisticEntity, removeDuplicateEntities, type OptimisticEntity } from "@/features/shared/optimistic"; +import { + createOptimisticEntity, + replaceOptimisticEntity, + removeDuplicateEntities, + type OptimisticEntity, +} from "@/features/shared/optimistic"; import { DISABLED_QUERY_KEY, STALE_TIMES } from "../../../shared/queryPatterns"; import { useSmartPolling } from "../../../ui/hooks"; import { useToast } from "../../../ui/hooks/useToast"; @@ -58,20 +63,18 @@ export function useCreateTask() { const previousTasks = queryClient.getQueryData(taskKeys.byProject(newTaskData.project_id)); // Create optimistic task with stable ID - const optimisticTask = createOptimisticEntity( - { - project_id: newTaskData.project_id, - title: newTaskData.title, - description: newTaskData.description || "", - status: newTaskData.status ?? "todo", - assignee: newTaskData.assignee ?? "User", - feature: newTaskData.feature, - task_order: newTaskData.task_order ?? 100, - priority: newTaskData.priority ?? "medium", - created_at: new Date().toISOString(), - updated_at: new Date().toISOString(), - } - ); + const optimisticTask = createOptimisticEntity({ + project_id: newTaskData.project_id, + title: newTaskData.title, + description: newTaskData.description || "", + status: newTaskData.status ?? "todo", + assignee: newTaskData.assignee ?? "User", + feature: newTaskData.feature, + task_order: newTaskData.task_order ?? 100, + priority: newTaskData.priority ?? "medium", + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }); // Optimistically add the new task queryClient.setQueryData(taskKeys.byProject(newTaskData.project_id), (old: Task[] | undefined) => { @@ -99,7 +102,7 @@ export function useCreateTask() { (tasks: (Task & Partial)[] = []) => { const replaced = replaceOptimisticEntity(tasks, context?.optimisticId || "", serverTask); return removeDuplicateEntities(replaced); - } + }, ); // Invalidate counts since we have a new task diff --git a/archon-ui-main/src/features/shared/optimistic.test.ts b/archon-ui-main/src/features/shared/optimistic.test.ts index ee435ac..4357239 100644 --- a/archon-ui-main/src/features/shared/optimistic.test.ts +++ b/archon-ui-main/src/features/shared/optimistic.test.ts @@ -38,7 +38,7 @@ describe("Optimistic Update Utilities", () => { it("should apply additional defaults", () => { const entity = createOptimisticEntity<{ id: string; name: string; status: string }>( { name: "Test" }, - { status: "pending" } + { status: "pending" }, ); expect(entity.status).toBe("pending"); @@ -107,4 +107,4 @@ describe("Optimistic Update Utilities", () => { expect("_localId" in cleaned).toBe(false); }); }); -}); \ No newline at end of file +}); diff --git a/archon-ui-main/src/features/shared/optimistic.ts b/archon-ui-main/src/features/shared/optimistic.ts index 48efa3a..b762aad 100644 --- a/archon-ui-main/src/features/shared/optimistic.ts +++ b/archon-ui-main/src/features/shared/optimistic.ts @@ -29,7 +29,7 @@ export function createOptimisticId(): string { */ export function createOptimisticEntity( data: Omit, - additionalDefaults?: Partial + additionalDefaults?: Partial, ): T & OptimisticEntity { const optimisticId = createOptimisticId(); return { @@ -48,7 +48,7 @@ export function createOptimisticEntity( export function replaceOptimisticEntity( entities: (T & Partial)[], localId: string, - serverEntity: T + serverEntity: T, ): T[] { return entities.map((entity) => { if ("_localId" in entity && entity._localId === localId) { @@ -79,4 +79,4 @@ export function removeDuplicateEntities(entities: T[]) export function cleanOptimisticMetadata(entity: T & Partial): T { const { _optimistic, _localId, ...cleaned } = entity; return cleaned as T; -} \ No newline at end of file +} diff --git a/archon-ui-main/src/features/shared/queryPatterns.ts b/archon-ui-main/src/features/shared/queryPatterns.ts index 0eee39b..c878a58 100644 --- a/archon-ui-main/src/features/shared/queryPatterns.ts +++ b/archon-ui-main/src/features/shared/queryPatterns.ts @@ -37,8 +37,8 @@ function getErrorStatus(error: unknown): number | undefined { const anyErr = error as any; // Check common status properties in order of likelihood - if (typeof anyErr.statusCode === "number") return anyErr.statusCode; // APIServiceError - if (typeof anyErr.status === "number") return anyErr.status; // fetch Response + if (typeof anyErr.statusCode === "number") return anyErr.statusCode; // APIServiceError + if (typeof anyErr.status === "number") return anyErr.status; // fetch Response if (typeof anyErr.response?.status === "number") return anyErr.response.status; // axios return undefined; diff --git a/archon-ui-main/src/features/shared/utils/clipboard.ts b/archon-ui-main/src/features/shared/utils/clipboard.ts index b7052bb..bf845d4 100644 --- a/archon-ui-main/src/features/shared/utils/clipboard.ts +++ b/archon-ui-main/src/features/shared/utils/clipboard.ts @@ -5,7 +5,7 @@ export interface ClipboardResult { success: boolean; - method: 'clipboard-api' | 'execCommand' | 'failed'; + method: "clipboard-api" | "execCommand" | "failed"; error?: string; } @@ -16,26 +16,22 @@ export interface ClipboardResult { */ export const copyToClipboard = async (text: string): Promise => { // Try modern clipboard API first with SSR-safe guards - if ( - typeof navigator !== 'undefined' && - navigator.clipboard && - navigator.clipboard.writeText - ) { + if (typeof navigator !== "undefined" && navigator.clipboard && navigator.clipboard.writeText) { try { await navigator.clipboard.writeText(text); - return { success: true, method: 'clipboard-api' }; + return { success: true, method: "clipboard-api" }; } catch (error) { - console.warn('Clipboard API failed, trying fallback:', error); + console.warn("Clipboard API failed, trying fallback:", error); } } // Fallback to document.execCommand for older browsers or insecure contexts // Add SSR guards for document access - if (typeof document === 'undefined') { + if (typeof document === "undefined") { return { success: false, - method: 'failed', - error: 'Running in server-side environment - clipboard not available' + method: "failed", + error: "Running in server-side environment - clipboard not available", }; } @@ -45,41 +41,41 @@ export const copyToClipboard = async (text: string): Promise => if (!document.body) { return { success: false, - method: 'failed', - error: 'document.body is not available' + method: "failed", + error: "document.body is not available", }; } - textarea = document.createElement('textarea'); + textarea = document.createElement("textarea"); textarea.value = text; - textarea.style.position = 'fixed'; - textarea.style.top = '-9999px'; - textarea.style.left = '-9999px'; - textarea.style.opacity = '0'; - textarea.style.pointerEvents = 'none'; - textarea.setAttribute('readonly', ''); - textarea.setAttribute('aria-hidden', 'true'); + textarea.style.position = "fixed"; + textarea.style.top = "-9999px"; + textarea.style.left = "-9999px"; + textarea.style.opacity = "0"; + textarea.style.pointerEvents = "none"; + textarea.setAttribute("readonly", ""); + textarea.setAttribute("aria-hidden", "true"); document.body.appendChild(textarea); textarea.select(); textarea.setSelectionRange(0, text.length); - const success = document.execCommand('copy'); + const success = document.execCommand("copy"); if (success) { - return { success: true, method: 'execCommand' }; + return { success: true, method: "execCommand" }; } else { return { success: false, - method: 'failed', - error: 'execCommand copy returned false' + method: "failed", + error: "execCommand copy returned false", }; } } catch (error) { return { success: false, - method: 'failed', - error: error instanceof Error ? error.message : 'Unknown error' + method: "failed", + error: error instanceof Error ? error.message : "Unknown error", }; } finally { // Always clean up the textarea element if it was created and added to DOM @@ -88,7 +84,7 @@ export const copyToClipboard = async (text: string): Promise => document.body.removeChild(textarea); } catch (cleanupError) { // Ignore cleanup errors - element may have already been removed - console.warn('Failed to cleanup textarea element:', cleanupError); + console.warn("Failed to cleanup textarea element:", cleanupError); } } } @@ -101,20 +97,17 @@ export const copyToClipboard = async (text: string): Promise => export const isClipboardSupported = (): boolean => { // Check modern clipboard API with proper SSR guards if ( - typeof navigator !== 'undefined' && - typeof navigator.clipboard !== 'undefined' && - typeof navigator.clipboard.writeText === 'function' + typeof navigator !== "undefined" && + typeof navigator.clipboard !== "undefined" && + typeof navigator.clipboard.writeText === "function" ) { return true; } // Check execCommand fallback with SSR guards - if ( - typeof document !== 'undefined' && - typeof document.queryCommandSupported === 'function' - ) { + if (typeof document !== "undefined" && typeof document.queryCommandSupported === "function") { try { - return document.queryCommandSupported('copy'); + return document.queryCommandSupported("copy"); } catch { return false; } @@ -129,11 +122,9 @@ export const isClipboardSupported = (): boolean => { * @returns string - Description of current security context */ export const getSecurityContext = (): string => { - if (typeof window === 'undefined') return 'server'; - if (window.isSecureContext) return 'secure'; - if (window.location.protocol === 'https:') return 'https'; - if (window.location.hostname === 'localhost' || - window.location.hostname === '127.0.0.1') return 'localhost'; - return 'insecure'; + if (typeof window === "undefined") return "server"; + if (window.isSecureContext) return "secure"; + if (window.location.protocol === "https:") return "https"; + if (window.location.hostname === "localhost" || window.location.hostname === "127.0.0.1") return "localhost"; + return "insecure"; }; - diff --git a/archon-ui-main/src/features/testing/test-utils.tsx b/archon-ui-main/src/features/testing/test-utils.tsx index a636312..b604406 100644 --- a/archon-ui-main/src/features/testing/test-utils.tsx +++ b/archon-ui-main/src/features/testing/test-utils.tsx @@ -11,10 +11,7 @@ import { TooltipProvider } from "../ui/primitives/tooltip"; */ export function renderWithProviders( ui: React.ReactElement, - { - queryClient = createTestQueryClient(), - ...renderOptions - } = {}, + { queryClient = createTestQueryClient(), ...renderOptions } = {}, ) { function Wrapper({ children }: { children: React.ReactNode }) { return ( diff --git a/archon-ui-main/src/features/ui/primitives/combobox.tsx b/archon-ui-main/src/features/ui/primitives/combobox.tsx index f9ca020..ecd36c7 100644 --- a/archon-ui-main/src/features/ui/primitives/combobox.tsx +++ b/archon-ui-main/src/features/ui/primitives/combobox.tsx @@ -247,11 +247,11 @@ export const ComboBox = React.forwardRef( aria-autocomplete="list" aria-activedescendant={ open - ? (hasCustomOption && highlightedIndex === filteredOptions.length - ? `${listboxId}-custom` - : highlightedIndex < filteredOptions.length + ? hasCustomOption && highlightedIndex === filteredOptions.length + ? `${listboxId}-custom` + : highlightedIndex < filteredOptions.length ? `${listboxId}-opt-${highlightedIndex}` - : undefined) + : undefined : undefined } value={search}