mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 15:03:57 +00:00
fix(flows): live-updating runs lists + row-action cleanup (enable toggle, delete in overflow) (#4890)
This commit is contained in:
@@ -50,13 +50,13 @@ describe('FlowListRow', () => {
|
||||
it('renders the flow name and reflects enabled state on the toggle', () => {
|
||||
renderRow();
|
||||
expect(screen.getByText('Daily digest')).toBeInTheDocument();
|
||||
// The toggle is an icon button; state is conveyed via aria-pressed, not text.
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-pressed', 'true');
|
||||
// The toggle is a SettingsSwitch (role=switch); state is conveyed via aria-checked.
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-checked', 'true');
|
||||
});
|
||||
|
||||
it('reflects paused state on the toggle when disabled', () => {
|
||||
renderRow({ flow: makeFlow({ enabled: false }) });
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-pressed', 'false');
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-checked', 'false');
|
||||
});
|
||||
|
||||
it('shows "Never run" when the flow has no last_run_at', () => {
|
||||
@@ -147,9 +147,16 @@ describe('FlowListRow', () => {
|
||||
expect(onDuplicate).toHaveBeenCalledWith(makeFlow());
|
||||
});
|
||||
|
||||
it('deletes via the direct Delete icon (not the menu)', () => {
|
||||
it('routes Delete through the overflow menu', () => {
|
||||
const { onDelete } = renderRow();
|
||||
fireEvent.click(screen.getByTestId('flow-delete-flow-1'));
|
||||
// Delete is a destructive secondary action now — behind the "⋯" menu, not
|
||||
// a standalone icon button in the flat row.
|
||||
expect(screen.queryByTestId('flow-delete-flow-1')).not.toBeInTheDocument();
|
||||
fireEvent.click(screen.getByTestId('flow-menu-flow-1'));
|
||||
|
||||
const deleteItem = screen.getByTestId('flow-delete-flow-1');
|
||||
expect(deleteItem).toHaveTextContent('Delete');
|
||||
fireEvent.click(deleteItem);
|
||||
expect(onDelete).toHaveBeenCalledWith(makeFlow());
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,14 +3,17 @@
|
||||
*
|
||||
* Mirrors the row layout of `CoreJobList`
|
||||
* (`app/src/components/settings/panels/cron/CoreJobList.tsx`): name + status
|
||||
* badge header, a line of run metadata, then a row of `Button` actions. Swaps
|
||||
* the cron "pause/resume" text button for a `SettingsSwitch` toggle (the
|
||||
* canonical boolean control — see `components/settings/controls`) since
|
||||
* enable/disable here is a persistent setting, not a one-off action.
|
||||
* badge header, a line of run metadata, then a row of `Button` actions. Uses
|
||||
* the canonical `SettingsSwitch` boolean control (`components/settings/controls`)
|
||||
* for enable/disable, since that's a persistent setting, not a one-off action —
|
||||
* not an icon `Button`, so its state reads at a glance instead of needing a
|
||||
* hover/title to disambiguate on vs. off.
|
||||
*
|
||||
* "View runs" (issue B5a.1) opens `FlowRunsDrawer` (mounted by `FlowsPage`)
|
||||
* for this flow's run history — re-added now that B3b's run inspector has
|
||||
* landed and the drawer has somewhere to send the user.
|
||||
* landed and the drawer has somewhere to send the user. Delete lives in the
|
||||
* same overflow menu (destructive actions shouldn't sit in the flat button
|
||||
* row next to Run/toggle, where a mis-click is one tap away).
|
||||
*
|
||||
* The flow name (issue B5b.1) is itself the "View" affordance for the new
|
||||
* read-only Workflow Canvas: it's rendered as a button that calls `onView`,
|
||||
@@ -22,6 +25,7 @@
|
||||
*/
|
||||
import { useT } from '../../lib/i18n/I18nContext';
|
||||
import type { Flow } from '../../services/api/flowsApi';
|
||||
import SettingsSwitch from '../settings/controls/SettingsSwitch';
|
||||
import Button from '../ui/Button';
|
||||
import FlowRowMenu from './FlowRowMenu';
|
||||
|
||||
@@ -33,41 +37,6 @@ function PlayIcon() {
|
||||
);
|
||||
}
|
||||
|
||||
function PowerIcon() {
|
||||
// On/off — enabled vs. paused (distinct from Run's play triangle).
|
||||
return (
|
||||
<svg
|
||||
className="h-4 w-4"
|
||||
viewBox="0 0 24 24"
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
strokeWidth={2}
|
||||
strokeLinecap="round"
|
||||
strokeLinejoin="round"
|
||||
aria-hidden="true">
|
||||
<path d="M12 2v10" />
|
||||
<path d="M18.36 6.64a9 9 0 11-12.73 0" />
|
||||
</svg>
|
||||
);
|
||||
}
|
||||
|
||||
function TrashIcon() {
|
||||
return (
|
||||
<svg
|
||||
className="h-4 w-4"
|
||||
viewBox="0 0 24 24"
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
strokeWidth={2}
|
||||
strokeLinecap="round"
|
||||
strokeLinejoin="round"
|
||||
aria-hidden="true">
|
||||
<path d="M3 6h18M8 6V4a1 1 0 011-1h6a1 1 0 011 1v2m2 0v14a1 1 0 01-1 1H7a1 1 0 01-1-1V6" />
|
||||
<path d="M10 11v6M14 11v6" />
|
||||
</svg>
|
||||
);
|
||||
}
|
||||
|
||||
/** Which of this row's actions currently has a request in flight, if any. */
|
||||
export type FlowListRowBusy = 'toggle' | 'run' | null;
|
||||
|
||||
@@ -153,27 +122,18 @@ const FlowListRow = ({
|
||||
</div>
|
||||
|
||||
{/* All controls sit together on the right: the toggle (enabled/paused —
|
||||
the switch alone communicates state), then Run, Delete, and an overflow
|
||||
menu with the secondary actions (view runs / export / duplicate). */}
|
||||
the switch alone communicates state), then Run, and an overflow menu
|
||||
with the secondary/destructive actions (view runs / export /
|
||||
duplicate / delete). */}
|
||||
<div className="flex flex-shrink-0 items-center gap-1.5">
|
||||
<Button
|
||||
type="button"
|
||||
variant="tertiary"
|
||||
size="sm"
|
||||
iconOnly
|
||||
data-testid={`flow-toggle-${flow.id}`}
|
||||
aria-label={t('flows.list.toggleEnabled')}
|
||||
aria-pressed={flow.enabled}
|
||||
title={flow.enabled ? t('flows.list.enabled') : t('flows.list.paused')}
|
||||
<SettingsSwitch
|
||||
id={`flow-switch-${flow.id}`}
|
||||
checked={flow.enabled}
|
||||
onCheckedChange={() => onToggle(flow)}
|
||||
disabled={toggleBusy}
|
||||
className={
|
||||
flow.enabled
|
||||
? 'text-sage-600 dark:text-sage-300'
|
||||
: 'text-content-faint hover:text-content-secondary'
|
||||
}
|
||||
onClick={() => onToggle(flow)}>
|
||||
<PowerIcon />
|
||||
</Button>
|
||||
aria-label={t('flows.list.toggleEnabled')}
|
||||
data-testid={`flow-toggle-${flow.id}`}
|
||||
/>
|
||||
<Button
|
||||
type="button"
|
||||
variant="primary"
|
||||
@@ -187,18 +147,6 @@ const FlowListRow = ({
|
||||
onClick={() => onRun(flow)}>
|
||||
<PlayIcon />
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
variant="tertiary"
|
||||
tone="danger"
|
||||
size="sm"
|
||||
iconOnly
|
||||
data-testid={`flow-delete-${flow.id}`}
|
||||
aria-label={t('flows.list.delete')}
|
||||
title={t('flows.list.delete')}
|
||||
onClick={() => onDelete(flow)}>
|
||||
<TrashIcon />
|
||||
</Button>
|
||||
<FlowRowMenu
|
||||
rowId={flow.id}
|
||||
items={[
|
||||
@@ -220,6 +168,13 @@ const FlowListRow = ({
|
||||
onSelect: () => onDuplicate(flow),
|
||||
testId: `flow-duplicate-${flow.id}`,
|
||||
},
|
||||
{
|
||||
key: 'delete',
|
||||
label: t('flows.list.delete'),
|
||||
onSelect: () => onDelete(flow),
|
||||
danger: true,
|
||||
testId: `flow-delete-${flow.id}`,
|
||||
},
|
||||
]}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
* `FlowRunInspectorDrawer.test.tsx`) so this suite only exercises the
|
||||
* run-history list + the nesting contract between the two drawers.
|
||||
*/
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { Provider } from 'react-redux';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
@@ -207,4 +207,80 @@ describe('FlowRunsDrawer', () => {
|
||||
fireEvent.keyDown(document, { key: 'Escape' });
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('discards a stale background refetch after the drawer flips to a different flow (race guard)', async () => {
|
||||
// Regression test for a codex review finding on this PR: the live-refresh
|
||||
// `refetch` in FlowRunsDrawer didn't guard against a response landing
|
||||
// after the drawer had already switched to a different flowId, so a slow
|
||||
// flow-A refetch could clobber flow-B's already-rendered runs.
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
let flowACalls = 0;
|
||||
let resolveStaleA: ((runs: FlowRun[]) => void) | undefined;
|
||||
listFlowRuns.mockImplementation((flowId: string) => {
|
||||
if (flowId === 'flow-a') {
|
||||
flowACalls += 1;
|
||||
if (flowACalls === 1) {
|
||||
// Initial load: one active run so the live-refresh poll subscribes.
|
||||
return Promise.resolve([
|
||||
makeRun({ id: 'run-a', flow_id: 'flow-a', status: 'running' }),
|
||||
]);
|
||||
}
|
||||
// The poll-triggered refetch — stays pending until resolved below,
|
||||
// simulating a slow response that outlives the flow switch.
|
||||
return new Promise<FlowRun[]>(resolve => {
|
||||
resolveStaleA = resolve;
|
||||
});
|
||||
}
|
||||
if (flowId === 'flow-b') {
|
||||
return Promise.resolve([
|
||||
makeRun({ id: 'run-b', flow_id: 'flow-b', status: 'completed' }),
|
||||
]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
const { rerender } = render(
|
||||
<Provider store={store}>
|
||||
<FlowRunsDrawer flowId="flow-a" onClose={vi.fn()} />
|
||||
</Provider>
|
||||
);
|
||||
|
||||
// Flush the initial load's already-resolved promise.
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId('flow-run-row-run-a')).toBeInTheDocument();
|
||||
|
||||
// Trigger the live-refresh poll fallback — issues the second, hanging
|
||||
// listFlowRuns('flow-a') call.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(5_000);
|
||||
});
|
||||
expect(flowACalls).toBe(2);
|
||||
|
||||
// Flip the drawer to a different flow while that refetch is still in flight.
|
||||
rerender(
|
||||
<Provider store={store}>
|
||||
<FlowRunsDrawer flowId="flow-b" onClose={vi.fn()} />
|
||||
</Provider>
|
||||
);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId('flow-run-row-run-b')).toBeInTheDocument();
|
||||
|
||||
// Now let the stale flow-a response land.
|
||||
await act(async () => {
|
||||
resolveStaleA?.([makeRun({ id: 'run-a-late', flow_id: 'flow-a', status: 'completed' })]);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
// Flow-b's runs must be unaffected by the late flow-a response.
|
||||
expect(screen.getByTestId('flow-run-row-run-b')).toBeInTheDocument();
|
||||
expect(screen.queryByTestId('flow-run-row-run-a-late')).not.toBeInTheDocument();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,10 +8,11 @@
|
||||
* to-close + Escape-to-close via `useEscapeKey`) so it renders as a fixed
|
||||
* overlay regardless of where the parent mounts it.
|
||||
*
|
||||
* Data is a one-shot fetch via `listFlowRuns` — no polling here. The run
|
||||
* inspector already polls a single run's live status via `useFlowRunPoller`;
|
||||
* polling the whole list here would duplicate that logic for no benefit
|
||||
* (the list only needs to be fresh when the drawer opens).
|
||||
* Data loads via `listFlowRuns` on open, then stays live via
|
||||
* {@link useFlowRunsLiveRefresh} while any run in the list is still active —
|
||||
* so a run stuck on "Running" here updates without the user having to close
|
||||
* and reopen the drawer. The run inspector separately polls a single run's
|
||||
* live status via `useFlowRunPoller`; that's unrelated to this list refresh.
|
||||
*
|
||||
* Clicking a run sets `selectedRunId` and renders the existing
|
||||
* `FlowRunInspectorDrawer` stacked on top: both are `fixed inset-0 z-50`
|
||||
@@ -24,9 +25,10 @@
|
||||
* single Escape press closes only the topmost overlay (the inspector) first.
|
||||
*/
|
||||
import debug from 'debug';
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
|
||||
import { useEscapeKey } from '../../hooks/useEscapeKey';
|
||||
import { useFlowRunsLiveRefresh } from '../../hooks/useFlowRunsLiveRefresh';
|
||||
import { useT } from '../../lib/i18n/I18nContext';
|
||||
import { type FlowRun, listFlowRuns } from '../../services/api/flowsApi';
|
||||
import {
|
||||
@@ -72,8 +74,15 @@ export function FlowRunsDrawer({ flowId, flowName, onClose, onFixWithAgent }: Pr
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [selectedRunId, setSelectedRunId] = useState<string | null>(null);
|
||||
// Tracks the flowId this drawer instance is *currently* showing, so an
|
||||
// in-flight `refetch()` started for a previous flow (see below) can detect
|
||||
// it's stale once the drawer flips to a new flowId and bail instead of
|
||||
// clobbering the new flow's already-loaded runs.
|
||||
const currentFlowIdRef = useRef(flowId);
|
||||
|
||||
useEffect(() => {
|
||||
currentFlowIdRef.current = flowId;
|
||||
|
||||
// Reset for the new target so a previous flow's runs/error can't linger
|
||||
// under a different flowId while the new fetch is in flight.
|
||||
setSelectedRunId(null);
|
||||
@@ -109,6 +118,31 @@ export function FlowRunsDrawer({ flowId, flowName, onClose, onFixWithAgent }: Pr
|
||||
};
|
||||
}, [flowId]);
|
||||
|
||||
// Background refresh for the live-update hook below — deliberately doesn't
|
||||
// touch `loading`/`error` so a poll tick or progress event never flashes
|
||||
// the loading state or clobbers a real load error with a transient one.
|
||||
// Guards against a stale response: if the drawer flips from flow A to flow
|
||||
// B while an A refetch is still in flight, the late A response must not
|
||||
// overwrite B's already-loaded runs (mirrors the `cancelled` guard on the
|
||||
// main load effect above).
|
||||
const refetch = useCallback(() => {
|
||||
if (!flowId) return;
|
||||
const requestFlowId = flowId;
|
||||
listFlowRuns(requestFlowId)
|
||||
.then(result => {
|
||||
if (currentFlowIdRef.current !== requestFlowId) return;
|
||||
setRuns(result);
|
||||
log('refetched runs: flowId=%s count=%d', requestFlowId, result.length);
|
||||
})
|
||||
.catch(err => {
|
||||
if (currentFlowIdRef.current !== requestFlowId) return;
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
log('refetch failed: flowId=%s err=%s', requestFlowId, msg);
|
||||
});
|
||||
}, [flowId]);
|
||||
|
||||
useFlowRunsLiveRefresh(runs, refetch);
|
||||
|
||||
useEscapeKey(
|
||||
() => {
|
||||
log('escape: closing flowId=%s', flowId);
|
||||
|
||||
@@ -3,8 +3,9 @@
|
||||
* dynamic left sidebar while a flow is open on the canvas (`/flows/:id`). A
|
||||
* compact, scannable run history (status dot + status + relative time); clicking
|
||||
* a run opens the full {@link FlowRunInspectorDrawer} (which polls its live
|
||||
* status). One-shot fetch via `listFlowRuns` with a manual refresh — the engine
|
||||
* emits no list-level socket events, so this mirrors `FlowRunsDrawer`'s model.
|
||||
* status). Fetches via `listFlowRuns`, with a manual refresh button plus
|
||||
* {@link useFlowRunsLiveRefresh} keeping the list itself live while any run
|
||||
* shown here is still active (no manual refresh/navigate-away required).
|
||||
*
|
||||
* Rendered by `FlowCanvasPage` inside a `SidebarContent` portal, so it only
|
||||
* appears for a persisted flow (a draft has no runs yet).
|
||||
@@ -13,6 +14,7 @@ import createDebug from 'debug';
|
||||
import { useCallback, useEffect, useState } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
|
||||
import { useFlowRunsLiveRefresh } from '../../hooks/useFlowRunsLiveRefresh';
|
||||
import { useT } from '../../lib/i18n/I18nContext';
|
||||
import { type FlowRun, listFlowRuns } from '../../services/api/flowsApi';
|
||||
import { CenteredLoadingState, ErrorBanner } from '../ui/LoadingState';
|
||||
@@ -98,6 +100,8 @@ export default function FlowRunsSidebar({ flowId }: FlowRunsSidebarProps) {
|
||||
void load();
|
||||
}, [load]);
|
||||
|
||||
useFlowRunsLiveRefresh(runs, load);
|
||||
|
||||
return (
|
||||
<div className="flex h-full flex-col" data-testid="flow-runs-sidebar">
|
||||
<div className="flex flex-shrink-0 items-center justify-between gap-2 px-3 py-2">
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
/**
|
||||
* useFlowRunsLiveRefresh — unit tests.
|
||||
*
|
||||
* Verifies: no subscription/poll when every run is terminal, subscribe +
|
||||
* poll when a run is active, a trailing-debounced refetch on a matching
|
||||
* `flow:run_progress`/`flow_run_progress` event, teardown once the runs
|
||||
* settle to all-terminal, and cleanup on unmount.
|
||||
*/
|
||||
import { act, renderHook } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { FlowRun } from '../../services/api/flowsApi';
|
||||
import { useFlowRunsLiveRefresh } from '../useFlowRunsLiveRefresh';
|
||||
|
||||
const handlers = vi.hoisted(() => new Map<string, Set<(data: unknown) => void>>());
|
||||
const on = vi.hoisted(() =>
|
||||
vi.fn((event: string, cb: (data: unknown) => void) => {
|
||||
const set = handlers.get(event) ?? new Set();
|
||||
set.add(cb);
|
||||
handlers.set(event, set);
|
||||
})
|
||||
);
|
||||
const off = vi.hoisted(() =>
|
||||
vi.fn((event: string, cb: (data: unknown) => void) => {
|
||||
handlers.get(event)?.delete(cb);
|
||||
})
|
||||
);
|
||||
vi.mock('../../services/socketService', () => ({ socketService: { on, off } }));
|
||||
|
||||
function emit(event: 'flow:run_progress' | 'flow_run_progress', payload: unknown) {
|
||||
act(() => {
|
||||
for (const cb of handlers.get(event) ?? []) cb(payload);
|
||||
});
|
||||
}
|
||||
|
||||
function makeRun(overrides: Partial<FlowRun> = {}): FlowRun {
|
||||
return {
|
||||
id: 'run-1',
|
||||
flow_id: 'flow-1',
|
||||
thread_id: 'run-1',
|
||||
status: 'running',
|
||||
started_at: '2026-01-01T00:00:00Z',
|
||||
steps: [],
|
||||
pending_approvals: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('useFlowRunsLiveRefresh', () => {
|
||||
beforeEach(() => {
|
||||
handlers.clear();
|
||||
on.mockClear();
|
||||
off.mockClear();
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('does not subscribe or poll when every run is terminal', () => {
|
||||
const refetch = vi.fn();
|
||||
renderHook(() =>
|
||||
useFlowRunsLiveRefresh(
|
||||
[makeRun({ status: 'completed' }), makeRun({ id: 'run-2', status: 'failed' })],
|
||||
refetch
|
||||
)
|
||||
);
|
||||
|
||||
expect(on).not.toHaveBeenCalled();
|
||||
|
||||
vi.advanceTimersByTime(20_000);
|
||||
expect(refetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('subscribes to both event aliases and polls on a fallback interval when a run is active', () => {
|
||||
const refetch = vi.fn();
|
||||
renderHook(() => useFlowRunsLiveRefresh([makeRun({ status: 'running' })], refetch));
|
||||
|
||||
expect(on).toHaveBeenCalledWith('flow:run_progress', expect.any(Function));
|
||||
expect(on).toHaveBeenCalledWith('flow_run_progress', expect.any(Function));
|
||||
|
||||
vi.advanceTimersByTime(5_000);
|
||||
expect(refetch).toHaveBeenCalledTimes(1);
|
||||
|
||||
vi.advanceTimersByTime(5_000);
|
||||
expect(refetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('debounces a burst of flow:run_progress events into a single trailing refetch', () => {
|
||||
const refetch = vi.fn();
|
||||
renderHook(() => useFlowRunsLiveRefresh([makeRun({ status: 'running' })], refetch));
|
||||
|
||||
// Keep every emit + the final debounce settle comfortably inside the
|
||||
// first 5s poll tick so the poll fallback doesn't also fire here — that
|
||||
// interplay is covered separately by the "poll fallback" test above.
|
||||
emit('flow:run_progress', { run_id: 'run-1', node_id: 'a', status: 'success' });
|
||||
vi.advanceTimersByTime(500);
|
||||
emit('flow:run_progress', { run_id: 'run-1', node_id: 'b', status: 'success' });
|
||||
vi.advanceTimersByTime(500);
|
||||
emit('flow_run_progress', { run_id: 'run-1', node_id: 'c', status: 'success' });
|
||||
|
||||
// Still within the 3s trailing window from the last event — no refetch yet.
|
||||
expect(refetch).not.toHaveBeenCalled();
|
||||
|
||||
vi.advanceTimersByTime(3_000);
|
||||
expect(refetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('tears down the subscription and poll once the runs settle to all-terminal', () => {
|
||||
const refetch = vi.fn();
|
||||
const { rerender } = renderHook(({ runs }) => useFlowRunsLiveRefresh(runs, refetch), {
|
||||
initialProps: { runs: [makeRun({ status: 'running' })] },
|
||||
});
|
||||
|
||||
expect(on).toHaveBeenCalledTimes(2);
|
||||
|
||||
rerender({ runs: [makeRun({ status: 'completed' })] });
|
||||
|
||||
expect(off).toHaveBeenCalledWith('flow:run_progress', expect.any(Function));
|
||||
expect(off).toHaveBeenCalledWith('flow_run_progress', expect.any(Function));
|
||||
|
||||
refetch.mockClear();
|
||||
vi.advanceTimersByTime(20_000);
|
||||
expect(refetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('cleans up the subscription, poll, and any pending debounce on unmount', () => {
|
||||
const refetch = vi.fn();
|
||||
const { unmount } = renderHook(() =>
|
||||
useFlowRunsLiveRefresh([makeRun({ status: 'running' })], refetch)
|
||||
);
|
||||
|
||||
emit('flow:run_progress', { run_id: 'run-1', node_id: 'a', status: 'success' });
|
||||
|
||||
unmount();
|
||||
|
||||
expect(off).toHaveBeenCalledWith('flow:run_progress', expect.any(Function));
|
||||
expect(off).toHaveBeenCalledWith('flow_run_progress', expect.any(Function));
|
||||
|
||||
refetch.mockClear();
|
||||
vi.advanceTimersByTime(20_000);
|
||||
expect(refetch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* useFlowRunsLiveRefresh — keeps a runs LIST fresh while any run in it is
|
||||
* still active, so "Running" doesn't go stale until the user manually
|
||||
* refreshes or navigates away.
|
||||
*
|
||||
* The backend's `FlowRunObserver` publishes `DomainEvent::FlowRunProgress` on
|
||||
* each finished step, and the core socket bridge re-emits it to the frontend
|
||||
* as both `flow:run_progress` and `flow_run_progress` (colon + underscore
|
||||
* aliases — see `useFlowRunProgress`, whose subscribe/teardown style this
|
||||
* mirrors). There is, however, no terminal/completion socket event today —
|
||||
* a run transitioning `running` -> `completed`/`failed`/etc. emits nothing —
|
||||
* so a step-progress subscription alone can't catch that final refresh. This
|
||||
* hook therefore layers a 5s poll fallback on top of the socket subscription:
|
||||
* the socket keeps the refetch snappy while steps are landing, and the poll
|
||||
* guarantees the terminal transition (which has no event) still gets picked
|
||||
* up within a few seconds.
|
||||
*
|
||||
* This is deliberately dumb about *which* run changed — callers pass the
|
||||
* list they already fetched and a `refetch` that reloads it; this hook just
|
||||
* decides *when* to call `refetch` again. `FlowRunsSidebar`, `FlowRunsDrawer`,
|
||||
* and `WorkflowRunsPage` all wire it onto their existing one-shot fetchers.
|
||||
*/
|
||||
import debug from 'debug';
|
||||
import { useEffect, useRef } from 'react';
|
||||
|
||||
import type { FlowRun, FlowRunStatus } from '../services/api/flowsApi';
|
||||
import { socketService } from '../services/socketService';
|
||||
|
||||
const log = debug('flows:runs-live-refresh');
|
||||
|
||||
/** Socket event aliases the core bridge emits (colon + underscore forms). */
|
||||
const EVENT_COLON = 'flow:run_progress';
|
||||
const EVENT_UNDERSCORE = 'flow_run_progress';
|
||||
|
||||
/** Statuses a run never leaves once reached — no further refetch needed for it. */
|
||||
const TERMINAL_STATUSES = new Set<FlowRunStatus>([
|
||||
'completed',
|
||||
'completed_with_warnings',
|
||||
'failed',
|
||||
'cancelled',
|
||||
]);
|
||||
|
||||
/** Trailing debounce window for a burst of `flow:run_progress` events. */
|
||||
const DEBOUNCE_MS = 3_000;
|
||||
|
||||
/** Poll fallback cadence — catches the terminal transition, which has no socket event. */
|
||||
const POLL_INTERVAL_MS = 5_000;
|
||||
|
||||
/**
|
||||
* Subscribes to live run-progress events (debounced) and polls on a fallback
|
||||
* interval while `runs` contains at least one non-terminal run, calling
|
||||
* `refetch` to reload the list. Subscribes to nothing — and tears down any
|
||||
* existing subscription/poll — once every run has settled or on unmount.
|
||||
*/
|
||||
export function useFlowRunsLiveRefresh(runs: FlowRun[], refetch: () => void): void {
|
||||
// Keep the latest `refetch` available to the effect without retriggering
|
||||
// subscribe/unsubscribe every time the caller passes a new closure.
|
||||
const refetchRef = useRef(refetch);
|
||||
refetchRef.current = refetch;
|
||||
|
||||
const hasActive = runs.some(run => !TERMINAL_STATUSES.has(run.status));
|
||||
|
||||
useEffect(() => {
|
||||
if (!hasActive) return;
|
||||
|
||||
let debounceTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
const scheduleRefetch = () => {
|
||||
if (debounceTimer) clearTimeout(debounceTimer);
|
||||
debounceTimer = setTimeout(() => {
|
||||
debounceTimer = null;
|
||||
log('debounced refetch firing');
|
||||
refetchRef.current();
|
||||
}, DEBOUNCE_MS);
|
||||
};
|
||||
|
||||
const handleProgress = () => {
|
||||
log('progress event received — scheduling debounced refetch');
|
||||
scheduleRefetch();
|
||||
};
|
||||
|
||||
log('subscribing: at least one active run');
|
||||
socketService.on(EVENT_COLON, handleProgress);
|
||||
socketService.on(EVENT_UNDERSCORE, handleProgress);
|
||||
|
||||
const pollId = setInterval(() => {
|
||||
log('poll fallback refetch');
|
||||
refetchRef.current();
|
||||
}, POLL_INTERVAL_MS);
|
||||
|
||||
return () => {
|
||||
log('tearing down: unsubscribing + clearing poll/debounce timers');
|
||||
socketService.off(EVENT_COLON, handleProgress);
|
||||
socketService.off(EVENT_UNDERSCORE, handleProgress);
|
||||
clearInterval(pollId);
|
||||
if (debounceTimer) clearTimeout(debounceTimer);
|
||||
};
|
||||
}, [hasActive]);
|
||||
}
|
||||
|
||||
export default useFlowRunsLiveRefresh;
|
||||
@@ -131,9 +131,9 @@ describe('FlowsPage', () => {
|
||||
fireEvent.click(screen.getByTestId('flow-toggle-flow-1'));
|
||||
|
||||
expect(setFlowEnabled).toHaveBeenCalledWith('flow-1', false);
|
||||
// The toggle is an icon button now; state is conveyed via aria-pressed.
|
||||
// The toggle is a SettingsSwitch (role=switch) now; state is conveyed via aria-checked.
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-pressed', 'false')
|
||||
expect(screen.getByTestId('flow-toggle-flow-1')).toHaveAttribute('aria-checked', 'false')
|
||||
);
|
||||
});
|
||||
|
||||
@@ -275,6 +275,8 @@ describe('FlowsPage', () => {
|
||||
deleteFlow.mockResolvedValue('flow-1');
|
||||
renderWithProviders(<FlowsPage />, { initialEntries: ['/?view=main'] });
|
||||
|
||||
// Delete now lives behind the row's "⋯" overflow menu, alongside
|
||||
// Export/Duplicate, rather than a standalone icon button.
|
||||
fireEvent.click(await screen.findByTestId('flow-menu-flow-1'));
|
||||
fireEvent.click(await screen.findByTestId('flow-delete-flow-1'));
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { act, render, screen, waitFor } from '@testing-library/react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import WorkflowRunsPage from './WorkflowRunsPage';
|
||||
|
||||
@@ -75,4 +75,67 @@ describe('WorkflowRunsPage', () => {
|
||||
|
||||
await waitFor(() => expect(screen.getByText('rpc down')).toBeInTheDocument());
|
||||
});
|
||||
|
||||
describe('live refresh (useFlowRunsLiveRefresh integration)', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('re-fetches just the runs (not listFlows) on the live-refresh poll while a run is active', async () => {
|
||||
vi.useFakeTimers();
|
||||
listAllFlowRuns
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'r1', flow_id: 'f1', status: 'running', started_at: '2026-01-01T00:00:00Z' },
|
||||
])
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'r1', flow_id: 'f1', status: 'completed', started_at: '2026-01-01T00:00:00Z' },
|
||||
]);
|
||||
listFlows.mockResolvedValue([{ id: 'f1', name: 'Daily digest' }]);
|
||||
|
||||
render(<WorkflowRunsPage />);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId('workflow-runs-list')).toBeInTheDocument();
|
||||
expect(listAllFlowRuns).toHaveBeenCalledTimes(1);
|
||||
expect(listFlows).toHaveBeenCalledTimes(1);
|
||||
|
||||
// The 5s poll fallback fires `refetchRuns` while the one run is still 'running'.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(5_000);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
// The poll fallback re-fetched just the runs — `listFlows` is not called again.
|
||||
expect(listAllFlowRuns).toHaveBeenCalledTimes(2);
|
||||
expect(listFlows).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not surface an error banner when a background refetch fails', async () => {
|
||||
vi.useFakeTimers();
|
||||
listAllFlowRuns
|
||||
.mockResolvedValueOnce([
|
||||
{ id: 'r1', flow_id: 'f1', status: 'running', started_at: '2026-01-01T00:00:00Z' },
|
||||
])
|
||||
.mockRejectedValueOnce(new Error('transient rpc blip'));
|
||||
listFlows.mockResolvedValue([{ id: 'f1', name: 'Daily digest' }]);
|
||||
|
||||
render(<WorkflowRunsPage />);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId('workflow-runs-list')).toBeInTheDocument();
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(5_000);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(listAllFlowRuns).toHaveBeenCalledTimes(2);
|
||||
// The transient background failure is logged only — the list stays as-is,
|
||||
// no error banner from the (unrelated) `load` error state.
|
||||
expect(screen.queryByText('transient rpc blip')).not.toBeInTheDocument();
|
||||
expect(screen.getByTestId('workflow-runs-list')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,13 +1,18 @@
|
||||
/**
|
||||
* WorkflowRunsPage — the aggregate "All runs" view: every workflow's runs across
|
||||
* the whole `flows` domain, newest first, backed by the `flows_list_all_runs`
|
||||
* core RPC. Each row links back to its workflow's canvas.
|
||||
* core RPC. Each row links back to its workflow's canvas. Stays live via
|
||||
* {@link useFlowRunsLiveRefresh} while any listed run is still active, via a
|
||||
* lightweight `refetchRuns` (re-fetches just the runs, not `listFlows()` too)
|
||||
* so a run doesn't sit on "Running" until the user reloads the page.
|
||||
*/
|
||||
import debug from 'debug';
|
||||
import { useCallback, useEffect, useState } from 'react';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
|
||||
import PanelPage from '../components/layout/PanelPage';
|
||||
import { CenteredLoadingState, ErrorBanner } from '../components/ui/LoadingState';
|
||||
import { useFlowRunsLiveRefresh } from '../hooks/useFlowRunsLiveRefresh';
|
||||
import { useT } from '../lib/i18n/I18nContext';
|
||||
import {
|
||||
type Flow,
|
||||
@@ -17,6 +22,8 @@ import {
|
||||
listFlows,
|
||||
} from '../services/api/flowsApi';
|
||||
|
||||
const log = debug('app:flows:runs-page');
|
||||
|
||||
const STATUS_CLASS: Record<FlowRunStatus, string> = {
|
||||
running: 'bg-primary-500/15 text-primary-600 dark:text-primary-300',
|
||||
completed: 'bg-sage-500/15 text-sage-700 dark:text-sage-300',
|
||||
@@ -56,6 +63,21 @@ export default function WorkflowRunsPage() {
|
||||
void load();
|
||||
}, [load]);
|
||||
|
||||
// Lighter-weight than `load` — only re-fetches the runs (not `listFlows()`
|
||||
// too), since flow names rarely change mid-run and re-fetching them on
|
||||
// every live-refresh tick would be wasted work.
|
||||
const refetchRuns = useCallback(() => {
|
||||
listAllFlowRuns()
|
||||
.then(setRuns)
|
||||
.catch(err => {
|
||||
// Best-effort background refresh — a transient failure here shouldn't
|
||||
// clobber the page's existing error/loading state from `load`.
|
||||
log('refetchRuns failed: %o', err);
|
||||
});
|
||||
}, []);
|
||||
|
||||
useFlowRunsLiveRefresh(runs, refetchRuns);
|
||||
|
||||
const statusLabel = (status: FlowRunStatus) =>
|
||||
t(`flows.allRuns.status.${status}`, status.replace(/_/g, ' '));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user