fix(global-search): surface unreachable nodes and harden the command palette (#1253)

* fix(global-search): surface unreachable nodes and harden the command palette The command palette discarded the cross-node search hook's failedNodes, so a search run while a fleet node was down silently returned partial results with no sign a host was skipped. It now renders an "N nodes unreachable" line and still shows the stacks it could gather. The shared cross-node search hook fetched every node's stack list and statuses on every keystroke. It now fans out once per search session and filters the cached inventory client-side as the query is refined, cutting per-keystroke fleet traffic. A 200 response with an unparseable status body degrades stacks to unknown instead of failing the whole node. The palette now owns result matching (cmdk's built-in fuzzy filter is disabled), so Pages, Nodes, and Stacks match by case-insensitive substring in a deterministic order and the 50-row cap applies to the real match set rather than a re-sorted slice. Adds unit coverage for the hook and palette, plus a Playwright journey spec. * fix(global-search): clear stale cross-node results when the active node changes When excludeNodeId changed mid-search (the sidebar switches it on active-node change), the hook started a fresh fanout but left the previous session's inventory and failedNodes visible until the refetch resolved, so the newly active node could briefly appear under the other-nodes results or a stale unreachable warning could persist. The new session now drops prior results synchronously before refetching.
2026-08-30 20:29:15 +00:00 · 2026-05-29 19:04:39 -04:00
parent d41282e352
commit 98049e3b1c
7 changed files with 731 additions and 53 deletions
@@ -0,0 +1,305 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { renderHook, act } from '@testing-library/react';
+import type { Node } from '@/context/NodeContext';
+
+const fetchForNodeMock = vi.fn();
+vi.mock('@/lib/api', () => ({
+  fetchForNode: (...args: unknown[]) => fetchForNodeMock(...args),
+}));
+
+const useNodesMock = vi.fn();
+vi.mock('@/context/NodeContext', () => ({
+  useNodes: () => useNodesMock(),
+}));
+
+import { useCrossNodeStackSearch } from '../useCrossNodeStackSearch';
+
+function node(id: number, name: string, status: Node['status'] = 'online'): Node {
+  return {
+    id,
+    name,
+    type: id === 1 ? 'local' : 'remote',
+    compose_dir: '/compose',
+    is_default: id === 1,
+    status,
+    created_at: 0,
+  };
+}
+
+function res(status: number, body: unknown): Response {
+  return new Response(JSON.stringify(body), {
+    status,
+    headers: { 'Content-Type': 'application/json' },
+  });
+}
+
+interface NodeFixture {
+  files?: string[];
+  statuses?: Record<string, unknown>;
+  listStatus?: number;
+  statusStatus?: number;
+  throwOn?: 'list' | 'both';
+}
+
+function mockFleet(map: Record<number, NodeFixture>) {
+  fetchForNodeMock.mockImplementation((endpoint: string, nodeId: number) => {
+    const cfg = map[nodeId] ?? {};
+    if (cfg.throwOn === 'both' || (cfg.throwOn === 'list' && endpoint === '/stacks')) {
+      return Promise.reject(new Error('boom'));
+    }
+    if (endpoint === '/stacks') {
+      return Promise.resolve(res(cfg.listStatus ?? 200, cfg.files ?? []));
+    }
+    if (endpoint === '/stacks/statuses') {
+      return Promise.resolve(res(cfg.statusStatus ?? 200, cfg.statuses ?? {}));
+    }
+    return Promise.resolve(res(404, {}));
+  });
+}
+
+async function flushDebounce(ms = 260) {
+  await act(async () => {
+    await vi.advanceTimersByTimeAsync(ms);
+  });
+}
+
+beforeEach(() => {
+  fetchForNodeMock.mockReset();
+  useNodesMock.mockReset();
+  vi.useFakeTimers();
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+  vi.clearAllMocks();
+});
+
+describe('useCrossNodeStackSearch', () => {
+  it('does not fetch while the query is empty', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local'), node(2, 'opsix')] });
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: '', enabled: true }));
+
+    await flushDebounce(300);
+
+    expect(fetchForNodeMock).not.toHaveBeenCalled();
+    expect(result.current.hits).toEqual([]);
+    expect(result.current.loading).toBe(false);
+  });
+
+  it('does not fetch when disabled even with a query', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local')] });
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'db', enabled: false }));
+
+    await flushDebounce();
+
+    expect(fetchForNodeMock).not.toHaveBeenCalled();
+    expect(result.current.hits).toEqual([]);
+  });
+
+  it('fans out once per search session and filters cached inventory on refine (no refetch)', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local'), node(2, 'opsix')] });
+    mockFleet({
+      1: { files: ['db.yml', 'web.yml'], statuses: { 'db.yml': { status: 'running' }, 'web.yml': { status: 'exited' } } },
+      2: { files: ['mariadb.yml', 'cache.yml'], statuses: { 'mariadb.yml': 'running' } },
+    });
+
+    const { result, rerender } = renderHook(
+      ({ query }) => useCrossNodeStackSearch({ query, enabled: true }),
+      { initialProps: { query: 'd' } },
+    );
+
+    await flushDebounce();
+
+    // /stacks + /stacks/statuses for each of 2 nodes = 4 calls.
+    expect(fetchForNodeMock).toHaveBeenCalledTimes(4);
+    expect(result.current.hits.map(h => h.file).sort()).toEqual(['db.yml', 'mariadb.yml']);
+    expect(result.current.hits.find(h => h.file === 'db.yml')?.status).toBe('running');
+    expect(result.current.hits.find(h => h.file === 'mariadb.yml')?.status).toBe('running');
+
+    // Refining to a term that matches a different file must be served from the
+    // cached inventory without any new network calls.
+    rerender({ query: 'web' });
+    await flushDebounce();
+
+    expect(fetchForNodeMock).toHaveBeenCalledTimes(4);
+    expect(result.current.hits.map(h => h.file)).toEqual(['web.yml']);
+    expect(result.current.hits[0]?.status).toBe('exited');
+  });
+
+  it('skips offline nodes and the excluded node', async () => {
+    useNodesMock.mockReturnValue({
+      nodes: [node(1, 'local'), node(2, 'opsix', 'offline'), node(3, 'edge')],
+    });
+    mockFleet({ 1: { files: ['a.yml'] }, 3: { files: ['a.yml'] } });
+
+    const { result } = renderHook(() =>
+      useCrossNodeStackSearch({ query: 'a', enabled: true, excludeNodeId: 1 }),
+    );
+
+    await flushDebounce();
+
+    // node 1 excluded, node 2 offline, only node 3 queried = 2 calls.
+    expect(fetchForNodeMock).toHaveBeenCalledTimes(2);
+    const queriedNodeIds = new Set(fetchForNodeMock.mock.calls.map(c => c[1]));
+    expect(queriedNodeIds).toEqual(new Set([3]));
+    expect(result.current.hits.every(h => h.nodeId === 3)).toBe(true);
+  });
+
+  it('does not fetch when every node is offline', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local', 'offline')] });
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'db', enabled: true }));
+
+    await flushDebounce();
+
+    expect(fetchForNodeMock).not.toHaveBeenCalled();
+    expect(result.current.hits).toEqual([]);
+    expect(result.current.loading).toBe(false);
+  });
+
+  it('records a failed node when the stack list response is not ok, keeping other nodes', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local'), node(2, 'opsix')] });
+    mockFleet({
+      1: { files: ['db.yml'], statuses: { 'db.yml': 'running' } },
+      2: { listStatus: 502 },
+    });
+
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'db', enabled: true }));
+
+    await flushDebounce();
+
+    expect(result.current.failedNodes).toEqual([
+      { nodeId: 2, nodeName: 'opsix', reason: 'list returned HTTP 502' },
+    ]);
+    expect(result.current.hits.map(h => h.file)).toEqual(['db.yml']);
+  });
+
+  it('records a failed node with the error message when the request throws', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(2, 'opsix')] });
+    mockFleet({ 2: { throwOn: 'both' } });
+
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'x', enabled: true }));
+
+    await flushDebounce();
+
+    expect(result.current.failedNodes).toEqual([
+      { nodeId: 2, nodeName: 'opsix', reason: 'boom' },
+    ]);
+    expect(result.current.hits).toEqual([]);
+  });
+
+  it('still lists stacks as unknown when only the status endpoint fails', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local')] });
+    mockFleet({ 1: { files: ['db.yml'], statusStatus: 500 } });
+
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'db', enabled: true }));
+
+    await flushDebounce();
+
+    // A status-endpoint failure is graceful degradation, not a node failure.
+    expect(result.current.failedNodes).toEqual([]);
+    expect(result.current.hits).toEqual([
+      { nodeId: 1, nodeName: 'local', file: 'db.yml', status: 'unknown' },
+    ]);
+  });
+
+  it('clears results when the search is disabled', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local')] });
+    mockFleet({ 1: { files: ['db.yml'], statuses: { 'db.yml': 'running' } } });
+
+    const { result, rerender } = renderHook(
+      ({ enabled }) => useCrossNodeStackSearch({ query: 'db', enabled }),
+      { initialProps: { enabled: true } },
+    );
+
+    await flushDebounce();
+    expect(result.current.hits.length).toBe(1);
+
+    rerender({ enabled: false });
+    await flushDebounce(10);
+
+    expect(result.current.hits).toEqual([]);
+  });
+
+  it('starts a fresh fanout after the query is cleared and re-entered', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local')] });
+    mockFleet({ 1: { files: ['db.yml', 'web.yml'], statuses: { 'db.yml': 'running' } } });
+
+    const { result, rerender } = renderHook(
+      ({ query }) => useCrossNodeStackSearch({ query, enabled: true }),
+      { initialProps: { query: 'db' } },
+    );
+    await flushDebounce();
+    expect(fetchForNodeMock).toHaveBeenCalledTimes(2);
+    expect(result.current.hits.map(h => h.file)).toEqual(['db.yml']);
+
+    // Clearing the query ends the session and clears results.
+    rerender({ query: '' });
+    await flushDebounce(10);
+    expect(result.current.hits).toEqual([]);
+
+    // Re-entering a query starts a new session, so the fleet is queried again.
+    rerender({ query: 'web' });
+    await flushDebounce();
+    expect(fetchForNodeMock).toHaveBeenCalledTimes(4);
+    expect(result.current.hits.map(h => h.file)).toEqual(['web.yml']);
+  });
+
+  it('re-fans-out when excludeNodeId changes (active node switch)', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local'), node(2, 'opsix')] });
+    mockFleet({ 1: { files: ['a.yml'] }, 2: { files: ['a.yml'] } });
+
+    const { rerender } = renderHook(
+      ({ excludeNodeId }) => useCrossNodeStackSearch({ query: 'a', enabled: true, excludeNodeId }),
+      { initialProps: { excludeNodeId: 1 } },
+    );
+    await flushDebounce();
+    expect(new Set(fetchForNodeMock.mock.calls.map(c => c[1]))).toEqual(new Set([2]));
+
+    fetchForNodeMock.mockClear();
+    rerender({ excludeNodeId: 2 });
+    await flushDebounce();
+    expect(new Set(fetchForNodeMock.mock.calls.map(c => c[1]))).toEqual(new Set([1]));
+  });
+
+  it('clears stale results immediately when excludeNodeId changes, before the refetch resolves', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local'), node(2, 'opsix'), node(3, 'edge')] });
+    mockFleet({ 1: { files: ['a.yml'] }, 2: { files: ['a.yml'] }, 3: { files: ['a.yml'] } });
+
+    const { result, rerender } = renderHook(
+      ({ excludeNodeId }) => useCrossNodeStackSearch({ query: 'a', enabled: true, excludeNodeId }),
+      { initialProps: { excludeNodeId: 1 } },
+    );
+    await flushDebounce();
+    expect(result.current.hits.map(h => h.nodeId).sort()).toEqual([2, 3]);
+
+    // Switching active node (excludeNodeId 1 -> 2) must drop the stale inventory
+    // at once, before the new fanout resolves, so node 2 does not linger.
+    rerender({ excludeNodeId: 2 });
+    expect(result.current.hits).toEqual([]);
+    expect(result.current.loading).toBe(true);
+
+    await flushDebounce();
+    expect(result.current.hits.map(h => h.nodeId).sort()).toEqual([1, 3]);
+  });
+
+  it('degrades to unknown when the status endpoint is 200 with an unparseable body', async () => {
+    useNodesMock.mockReturnValue({ nodes: [node(1, 'local')] });
+    fetchForNodeMock.mockImplementation((endpoint: string) => {
+      if (endpoint === '/stacks') return Promise.resolve(res(200, ['db.yml']));
+      // 200 OK but a non-JSON body, so statusRes.json() throws.
+      return Promise.resolve(new Response('<html>not json</html>', {
+        status: 200,
+        headers: { 'Content-Type': 'text/html' },
+      }));
+    });
+
+    const { result } = renderHook(() => useCrossNodeStackSearch({ query: 'db', enabled: true }));
+    await flushDebounce();
+
+    // The successful stack list must survive a bad status body.
+    expect(result.current.failedNodes).toEqual([]);
+    expect(result.current.hits).toEqual([
+      { nodeId: 1, nodeName: 'local', file: 'db.yml', status: 'unknown' },
+    ]);
+  });
+});
@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';
 import { useNodes } from '@/context/NodeContext';
 import { fetchForNode } from '@/lib/api';

@@ -36,7 +36,10 @@ interface NodeOutcome {

 export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Options) {
    const { nodes } = useNodes();
-    const [hits, setHits] = useState<StackHit[]>([]);
+    // Full per-node inventory captured for the active search session. The query
+    // filter is applied client-side (see `hits` below) so refining the search
+    // never re-fans-out to the fleet.
+    const [inventory, setInventory] = useState<StackHit[]>([]);
    const [failedNodes, setFailedNodes] = useState<FailedNode[]>([]);
    const [loading, setLoading] = useState(false);

@@ -44,10 +47,15 @@ export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Optio
    const nodesRef = useRef(nodes);
    nodesRef.current = nodes;

+    const q = query.trim().toLowerCase();
+    // A search "session" is active while enabled and the query is non-empty. The
+    // fanout fetch runs once per session start, not per keystroke; clearing the
+    // query and typing again starts a fresh session (and a fresh fetch).
+    const active = enabled && q.length > 0;
+
    useEffect(() => {
-        const q = query.trim().toLowerCase();
-        if (!enabled || !q) {
-            setHits([]);
+        if (!active) {
+            setInventory([]);
            setFailedNodes([]);
            setLoading(false);
            return;
@@ -56,13 +64,21 @@ export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Optio
            n => n.status !== 'offline' && n.id !== excludeNodeId,
        );
        if (targets.length === 0) {
-            setHits([]);
+            setInventory([]);
            setFailedNodes([]);
+            setLoading(false);
            return;
        }
+        // Starting a new fetch session (a search became active, or the active
+        // node changed via excludeNodeId): drop the previous session's results
+        // so a stale inventory or unreachable warning never lingers during the
+        // refetch window. Refining the query keeps `active` true and does not
+        // re-run this effect, so the cached inventory survives keystrokes.
+        setLoading(true);
+        setInventory([]);
+        setFailedNodes([]);
        const controller = new AbortController();
        const timer = setTimeout(async () => {
-            setLoading(true);
            try {
                const perNode = await Promise.all(targets.map(async (node): Promise<NodeOutcome> => {
                    try {
@@ -81,26 +97,33 @@ export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Optio
                            };
                        }
                        const rawList = await listRes.json();
-                        const files: string[] = Array.isArray(rawList) ? rawList : [];
+                        const files: string[] = Array.isArray(rawList)
+                            ? rawList.filter((f): f is string => typeof f === 'string')
+                            : [];
                        const statuses: Record<string, StackStatus> = {};
                        if (statusRes.ok) {
-                            const raw = await statusRes.json();
-                            for (const [key, val] of Object.entries(raw)) {
-                                if (typeof val === 'string') {
-                                    statuses[key] = val as StackStatus;
-                                } else if (val && typeof val === 'object' && 'status' in val) {
-                                    statuses[key] = (val as StackStatusInfo).status;
+                            try {
+                                const raw = await statusRes.json();
+                                for (const [key, val] of Object.entries(raw)) {
+                                    if (typeof val === 'string') {
+                                        statuses[key] = val as StackStatus;
+                                    } else if (val && typeof val === 'object' && 'status' in val) {
+                                        statuses[key] = (val as StackStatusInfo).status;
+                                    }
                                }
+                            } catch {
+                                // A 200 with an unparseable status body must not discard the
+                                // stack list we already fetched; leave statuses empty so every
+                                // stack degrades to 'unknown' rather than failing the node.
                            }
                        }
-                        const nodeHits = files
-                            .filter(f => f.toLowerCase().includes(q))
-                            .map<StackHit>(file => ({
-                                nodeId: node.id,
-                                nodeName: node.name,
-                                file,
-                                status: statuses[file] ?? 'unknown',
-                            }));
+                        // Capture the whole node inventory; filtering is client-side.
+                        const nodeHits = files.map<StackHit>(file => ({
+                            nodeId: node.id,
+                            nodeName: node.name,
+                            file,
+                            status: statuses[file] ?? 'unknown',
+                        }));
                        return { hits: nodeHits, failure: null };
                    } catch (err) {
                        // AbortError is expected when the effect cleans up; don't
@@ -119,7 +142,7 @@ export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Optio
                    }
                }));
                if (controller.signal.aborted) return;
-                setHits(perNode.flatMap(o => o.hits));
+                setInventory(perNode.flatMap(o => o.hits));
                setFailedNodes(perNode.flatMap(o => (o.failure ? [o.failure] : [])));
            } finally {
                if (!controller.signal.aborted) setLoading(false);
@@ -129,7 +152,13 @@ export function useCrossNodeStackSearch({ query, enabled, excludeNodeId }: Optio
            clearTimeout(timer);
            controller.abort();
        };
-    }, [enabled, query, excludeNodeId]);
+    }, [active, excludeNodeId]);
+
+    // Client-side query filter over the session inventory. No network on keystrokes.
+    const hits = useMemo(
+        () => (active ? inventory.filter(h => h.file.toLowerCase().includes(q)) : []),
+        [active, inventory, q],
+    );

    return { hits, failedNodes, loading };
 }