Merge pull request #926 from MarvyNwaokobia/fix/590-631-633-663-multi-issue

fix/feat: resolve race condition, memory leak, add skeleton loading & error boundary

Author: leojay-net

dex_with_fiat_frontend/src/components/AuditTable.test.tsx

@@ -12,6 +12,50 @@ describe('AuditTable', () => {
     vi.restoreAllMocks();
   });
 
+  it('renders skeleton rows while loading and hides them once data arrives', async () => {
+    let resolveFetch!: (value: unknown) => void;
+    const fetchPromise = new Promise((resolve) => { resolveFetch = resolve; });
+
+    vi.stubGlobal('fetch', vi.fn(() => fetchPromise));
+
+    render(React.createElement(AuditTable));
+
+    // While the fetch is in-flight the table should be in aria-busy state
+    const busyTable = await waitFor(() => screen.getByRole('table', { name: /loading audit entries/i }));
+    expect(busyTable).toHaveAttribute('aria-busy', 'true');
+
+    // Skeleton cells are present (5 rows × 6 cells = 30 skeleton divs)
+    const skeletonCells = busyTable.querySelectorAll('td');
+    expect(skeletonCells.length).toBe(30);
+
+    // Resolve the fetch with real data
+    resolveFetch({
+      ok: true,
+      status: 200,
+      json: async () => ({
+        entries: [
+          {
+            id: 'e1',
+            timestamp: new Date().toISOString(),
+            adminAddress: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF',
+            actionType: 'deposit',
+            actionDescription: 'real-row',
+            txHash: 'abc123',
+            status: 'success',
+          },
+        ],
+        total: 1,
+      }),
+    });
+
+    await waitFor(() => {
+      expect(screen.getByText('real-row')).toBeInTheDocument();
+    });
+
+    // The skeleton table should no longer be in the DOM
+    expect(screen.queryByRole('table', { name: /loading audit entries/i })).not.toBeInTheDocument();
+  });
+
   it('does not apply stale fetch results after a newer request (abort)', async () => {
     vi.stubGlobal(
       'fetch',

dex_with_fiat_frontend/src/components/AuditTable.tsx

@@ -4,6 +4,7 @@ import { useState, useEffect, useCallback, useRef } from 'react';
 import { AuditEntry } from '@/types';
 import { useOnlineStatus } from '@/hooks/useOnlineStatus';
 import { useToast } from '@/hooks/useToast';
+import Skeleton from '@/components/ui/skeleton/Skeleton';
 
 interface AuditTableProps {
   onRefresh?: () => void;
@@ -330,7 +331,32 @@ export default function AuditTable({}: AuditTableProps) {
 
       {/* Audit Table */}
       <div className="bg-white dark:bg-gray-800 rounded-lg shadow overflow-x-auto">
-        {entries.length === 0 && !loading ? (
+        {loading ? (
+          <table className="w-full" aria-label="Loading audit entries" aria-busy="true">
+            <thead className="bg-gray-50 dark:bg-gray-700 border-b border-gray-200 dark:border-gray-600">
+              <tr>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">Timestamp</th>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">Admin</th>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">Action</th>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">Description</th>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">TX Hash</th>
+                <th className="px-6 py-3 text-left text-xs font-medium text-gray-700 dark:text-gray-300 uppercase tracking-wider">Status</th>
+              </tr>
+            </thead>
+            <tbody className="divide-y divide-gray-200 dark:divide-gray-700">
+              {Array.from({ length: 5 }).map((_, i) => (
+                <tr key={i} className="animate-pulse">
+                  <td className="px-6 py-4"><Skeleton className="h-4 w-36" /></td>
+                  <td className="px-6 py-4"><Skeleton className="h-4 w-24" /></td>
+                  <td className="px-6 py-4"><Skeleton className="h-4 w-20" /></td>
+                  <td className="px-6 py-4"><Skeleton className="h-4 w-48" /></td>
+                  <td className="px-6 py-4"><Skeleton className="h-4 w-20" /></td>
+                  <td className="px-6 py-4"><Skeleton className="h-6 w-16 rounded-full" /></td>
+                </tr>
+              ))}
+            </tbody>
+          </table>
+        ) : entries.length === 0 ? (
           <div className="p-8 text-center text-gray-500 dark:text-gray-400">
             <p className="text-lg mb-2">No audit entries found</p>
             <p className="text-sm">Try adjusting your filters or check back later</p>

dex_with_fiat_frontend/src/components/ChatHistorySidebar.tsx

@@ -5,6 +5,7 @@ import jsPDF from 'jspdf';
 import { useChatHistory } from '@/hooks/useChatHistory';
 import { useTxHistory } from '@/hooks/useTxHistory';
 import { useStellarWallet } from '@/contexts/StellarWalletContext';
+import ErrorBoundary from '@/components/ErrorBoundary';
 import {
   MessageSquare,
   Trash2,
@@ -460,6 +461,11 @@ export default function ChatHistorySidebar({
   };
 
   return (
+    <ErrorBoundary
+      title="Sidebar unavailable"
+      message="An unexpected error occurred in the chat history panel. Your conversations are safe."
+      retryLabel="Reload sidebar"
+    >
     <div
       className={`theme-surface theme-border h-full flex flex-col transition-all duration-300 border-r ${
         isCollapsed ? 'w-20' : 'w-full'
@@ -835,5 +841,6 @@ export default function ChatHistorySidebar({
         </div>
       )}
     </div>
+    </ErrorBoundary>
   );
 }

dex_with_fiat_frontend/src/components/__tests__/ChatHistorySidebar.test.tsx

@@ -244,3 +244,68 @@ describe('ChatHistorySidebar', () => {
     expect(screen.queryByText('History cleared')).toBeNull();
   });
 });
+
+// ── Issue #633 regression: error boundary wraps ChatHistorySidebar ─────────────
+describe('ChatHistorySidebar error boundary (#633)', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    global.fetch = vi.fn().mockResolvedValue({
+      ok: true,
+      json: async () => ({ events: [] }),
+    } as Response);
+    mockPinnedSessions = [];
+    mockUnpinnedSessions = [];
+    mockAllSessions = [];
+    // Suppress React's error boundary console.error during tests
+    vi.spyOn(console, 'error').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.restoreAllMocks();
+    vi.useRealTimers();
+  });
+
+  it('renders the fallback UI when a child throws and not the crash stack', async () => {
+    // Force PriceTicker (rendered inside the sidebar) to throw
+    vi.doMock('@/components/PriceTicker', () => ({
+      default: () => { throw new Error('PriceTicker exploded'); },
+    }));
+
+    // Dynamically import so the new mock is picked up
+    const { default: ChatHistorySidebarFresh } = await import('@/components/ChatHistorySidebar');
+
+    act(() => {
+      render(
+        <ChatHistorySidebarFresh onLoadSession={vi.fn()} isCollapsed={false} />,
+      );
+    });
+
+    await act(async () => { vi.advanceTimersByTime(900); });
+
+    expect(screen.getByText('Sidebar unavailable')).toBeTruthy();
+    expect(screen.queryByText('PriceTicker exploded')).toBeNull();
+
+    vi.doUnmock('@/components/PriceTicker');
+  });
+
+  it('displays the custom retry label from the error boundary props', async () => {
+    vi.doMock('@/components/PriceTicker', () => ({
+      default: () => { throw new Error('forced'); },
+    }));
+
+    const { default: ChatHistorySidebarFresh } = await import('@/components/ChatHistorySidebar');
+
+    act(() => {
+      render(
+        <ChatHistorySidebarFresh onLoadSession={vi.fn()} isCollapsed={false} />,
+      );
+    });
+
+    await act(async () => { vi.advanceTimersByTime(900); });
+
+    expect(screen.getByRole('button', { name: /reload sidebar/i })).toBeTruthy();
+
+    vi.doUnmock('@/components/PriceTicker');
+  });
+});

dex_with_fiat_frontend/src/hooks/chatStateMachine.test.ts

@@ -667,6 +667,48 @@ describe('ChatStateMachine', () => {
   });
 });
 
+// ── Issue #590 regression: shared INITIAL_CONTEXT must not be mutated ─────────
+describe('chatStateMachine race condition regression (#590)', () => {
+  it('two independent machines do not share context state', () => {
+    const machineA = createChatStateMachine();
+    machineA.transition(ChatEvent.INITIALIZE_SESSION);
+    machineA.updateContext({ messageCount: 7, hasUserCancelled: true });
+
+    const machineB = createChatStateMachine();
+    machineB.transition(ChatEvent.INITIALIZE_SESSION);
+
+    // Machine B must start clean — not polluted by machine A's mutations
+    expect(machineB.getState().context.messageCount).toBe(0);
+    expect(machineB.getState().context.hasUserCancelled).toBe(false);
+  });
+
+  it('action callbacks on machine A do not corrupt machine B context', () => {
+    const machineA = createChatStateMachine();
+    machineA.transition(ChatEvent.INITIALIZE_SESSION);
+    machineA.transition(ChatEvent.SEND_MESSAGE);
+    machineA.transition(ChatEvent.ANALYSIS_COMPLETE); // → ANALYZING
+    machineA.transition(ChatEvent.ENCOUNTER_ERROR);   // → ERROR
+    machineA.updateContext({ errorMessage: 'A failed' });
+
+    const machineB = createChatStateMachine();
+    machineB.transition(ChatEvent.INITIALIZE_SESSION);
+
+    expect(machineB.getState().context.errorMessage).toBeNull();
+    expect(machineB.getState().state).toBe(ChatState.INITIALIZED);
+  });
+
+  it('many machines created in sequence all start with zero messageCount', () => {
+    for (let i = 0; i < 5; i++) {
+      const m = createChatStateMachine();
+      m.transition(ChatEvent.INITIALIZE_SESSION);
+      m.updateContext({ messageCount: i + 10 });
+      const fresh = createChatStateMachine();
+      fresh.transition(ChatEvent.INITIALIZE_SESSION);
+      expect(fresh.getState().context.messageCount).toBe(0);
+    }
+  });
+});
+
 describe('chatStateMachine clipboard snapshot helpers', () => {
   it('formats a stable snapshot string', () => {
     const context: ChatMachineContext = {

dex_with_fiat_frontend/src/hooks/chatStateMachine.ts

@@ -76,7 +76,7 @@ const INITIAL_CONTEXT: ChatMachineContext = {
   needsClarification: false,
   clarificationQuestion: null,
   errorMessage: null,
-  lastEventTime: Date.now(),
+  lastEventTime: 0,
   previousState: null,
 };
 
@@ -128,13 +128,29 @@ class ChatGuards {
   };
 }
 
+/**
+ * Returns a fresh context object — never mutate the module-level constant directly.
+ */
+function getInitialContext(): ChatMachineContext {
+  return {
+    messageCount: 0,
+    hasUserCancelled: false,
+    pendingTransactionData: null,
+    needsClarification: false,
+    clarificationQuestion: null,
+    errorMessage: null,
+    lastEventTime: Date.now(),
+    previousState: null,
+  };
+}
+
 /**
  * Create and configure the chat state machine
  */
 export function createChatStateMachine(): StateMachine<ChatState, ChatEvent, ChatMachineContext> {
   const config: StateMachineConfig<ChatState, ChatEvent, ChatMachineContext> = {
     initial: ChatState.UNINITIALIZED,
-    context: INITIAL_CONTEXT,
+    context: getInitialContext(),
     states: {
       [ChatState.UNINITIALIZED]: {
         [ChatEvent.INITIALIZE_SESSION]: {

dex_with_fiat_frontend/src/hooks/useChat.test.ts

@@ -890,3 +890,82 @@ describe('useChat race condition regression (#530)', () => {
     harness.cleanup();
   });
 });
+
+// ── Issue #663 regression: memory leak in useChat ─────────────────────────────
+describe('useChat memory leak regression (#663)', () => {
+  beforeEach(() => {
+    analyzeQueue = [];
+    createNewSessionSpy.mockClear();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it('cancelled message uses a unique ID — no Date.now()+1 collision', async () => {
+    analyzeQueue = [new Promise<AnalyzeResult>(() => {})];
+
+    const harness = await setupHook();
+    await flushEffects(1);
+
+    act(() => { void harness.api.sendMessage('slow'); });
+    act(() => { harness.api.cancelPendingRequest(); });
+
+    await flushEffects(2);
+
+    const ids = harness.api.messages.map((m) => m.id);
+    const unique = new Set(ids);
+    expect(unique.size).toBe(ids.length);
+
+    harness.cleanup();
+  });
+
+  it('onTransactionReady is NOT called after component unmounts', async () => {
+    analyzeQueue = [
+      {
+        intent: 'fiat_conversion',
+        confidence: 0.95,
+        extractedData: { tokenIn: 'XLM' },
+        requiredQuestions: [],
+        suggestedResponse: 'ok',
+      },
+      {
+        intent: 'fiat_conversion',
+        confidence: 0.95,
+        extractedData: { fiatCurrency: 'NGN' },
+        requiredQuestions: [],
+        suggestedResponse: 'ok',
+      },
+      {
+        intent: 'fiat_conversion',
+        confidence: 0.95,
+        extractedData: { amountIn: '5' },
+        requiredQuestions: [],
+        suggestedResponse: 'ok',
+      },
+    ];
+
+    const harness = await setupHook();
+    vi.useFakeTimers();
+
+    const onReady = vi.fn();
+    act(() => { harness.api.setTransactionReadyCallback(onReady); });
+
+    await flushEffects(1);
+    await act(async () => { await harness.api.sendMessage('deposit'); });
+    await flushEffects(1);
+    await act(async () => { await harness.api.sendMessage('NGN'); });
+    await flushEffects(1);
+    await act(async () => { await harness.api.sendMessage('5'); });
+    await flushEffects(1);
+
+    // Unmount BEFORE the 1-second timer fires
+    harness.cleanup();
+
+    // Advance past the timer window
+    act(() => { vi.advanceTimersByTime(2000); });
+
+    // The callback must NOT have been called after unmount
+    expect(onReady).not.toHaveBeenCalled();
+  });
+});