From 1feb87317d06709f47a2dc8a25ac86d0fd76abf7 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Thu, 16 Jul 2026 02:24:17 +0530 Subject: [PATCH] fix(flows): show "awaiting approval" state in runs lists when a run is halted at an approval gate (#4932) --- .../components/flows/FlowRunsDrawer.test.tsx | 39 ++++ app/src/components/flows/FlowRunsDrawer.tsx | 12 +- .../components/flows/FlowRunsSidebar.test.tsx | 33 +++ app/src/components/flows/FlowRunsSidebar.tsx | 54 +++-- .../useRunsPendingApprovalSet.test.ts | 214 ++++++++++++++++++ app/src/hooks/useRunsPendingApprovalSet.ts | 118 ++++++++++ app/src/pages/WorkflowRunsPage.test.tsx | 42 ++++ app/src/pages/WorkflowRunsPage.tsx | 58 +++-- 8 files changed, 519 insertions(+), 51 deletions(-) create mode 100644 app/src/hooks/__tests__/useRunsPendingApprovalSet.test.ts create mode 100644 app/src/hooks/useRunsPendingApprovalSet.ts diff --git a/app/src/components/flows/FlowRunsDrawer.test.tsx b/app/src/components/flows/FlowRunsDrawer.test.tsx index 43cf8096a..d762ff680 100644 --- a/app/src/components/flows/FlowRunsDrawer.test.tsx +++ b/app/src/components/flows/FlowRunsDrawer.test.tsx @@ -23,6 +23,9 @@ import { FlowRunsDrawer } from './FlowRunsDrawer'; const listFlowRuns = vi.hoisted(() => vi.fn()); vi.mock('../../services/api/flowsApi', () => ({ listFlowRuns })); +const fetchPendingApprovals = vi.hoisted(() => vi.fn()); +vi.mock('../../services/api/approvalApi', () => ({ fetchPendingApprovals })); + const FlowRunInspectorDrawer = vi.hoisted(() => vi.fn()); vi.mock('./FlowRunInspectorDrawer', () => ({ FLOW_RUN_STATUS_ACCENT: { @@ -87,6 +90,7 @@ function renderDrawer(flowId: string | null, onClose: () => void, flowName?: str describe('FlowRunsDrawer', () => { beforeEach(() => { vi.clearAllMocks(); + fetchPendingApprovals.mockResolvedValue([]); }); it('renders null when flowId is null', () => { @@ -123,6 +127,41 @@ describe('FlowRunsDrawer', () => { expect(row).toHaveTextContent('Completed with warnings'); }); + it('shows "Awaiting approval" for a running run halted at a matching flow approval gate', async () => { + listFlowRuns.mockResolvedValue([makeRun({ id: 'run-1', status: 'running' })]); + fetchPendingApprovals.mockResolvedValue([ + { + request_id: 'req-1', + tool_name: 'SLACK_SEND_MESSAGE', + action_summary: 'Send Slack message', + args_redacted: {}, + session_id: 'session-1', + created_at: '2026-01-01T00:00:00Z', + expires_at: null, + source_context: { kind: 'flow', flow_id: 'flow-1', run_id: 'run-1' }, + }, + ]); + renderDrawer('flow-1', vi.fn()); + + const row = await screen.findByTestId('flow-run-row-run-1'); + await waitFor(() => expect(row).toHaveTextContent('Awaiting approval')); + expect(screen.getByTestId('flow-run-row-dot-run-1').className.includes('dot-pending')).toBe( + true + ); + }); + + it('leaves a running run without a matching flow approval labeled "Running"', async () => { + listFlowRuns.mockResolvedValue([makeRun({ id: 'run-1', status: 'running' })]); + fetchPendingApprovals.mockResolvedValue([]); + renderDrawer('flow-1', vi.fn()); + + const row = await screen.findByTestId('flow-run-row-run-1'); + await waitFor(() => expect(row).toHaveTextContent('Running')); + expect(screen.getByTestId('flow-run-row-dot-run-1').className.includes('dot-running')).toBe( + true + ); + }); + it('falls back to a generic title when no flowName is given', async () => { listFlowRuns.mockResolvedValue([]); renderDrawer('flow-1', vi.fn()); diff --git a/app/src/components/flows/FlowRunsDrawer.tsx b/app/src/components/flows/FlowRunsDrawer.tsx index 8f4484816..368c73b77 100644 --- a/app/src/components/flows/FlowRunsDrawer.tsx +++ b/app/src/components/flows/FlowRunsDrawer.tsx @@ -29,6 +29,10 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { useEscapeKey } from '../../hooks/useEscapeKey'; import { useFlowRunsLiveRefresh } from '../../hooks/useFlowRunsLiveRefresh'; +import { + resolveDisplayStatus, + useRunsPendingApprovalSet, +} from '../../hooks/useRunsPendingApprovalSet'; import { useT } from '../../lib/i18n/I18nContext'; import { type FlowRun, listFlowRuns } from '../../services/api/flowsApi'; import { @@ -142,6 +146,7 @@ export function FlowRunsDrawer({ flowId, flowName, onClose, onFixWithAgent }: Pr }, [flowId]); useFlowRunsLiveRefresh(runs, refetch); + const pendingRunIds = useRunsPendingApprovalSet(runs); useEscapeKey( () => { @@ -213,6 +218,7 @@ export function FlowRunsDrawer({ flowId, flowName, onClose, onFixWithAgent }: Pr diff --git a/app/src/hooks/__tests__/useRunsPendingApprovalSet.test.ts b/app/src/hooks/__tests__/useRunsPendingApprovalSet.test.ts new file mode 100644 index 000000000..80abba31e --- /dev/null +++ b/app/src/hooks/__tests__/useRunsPendingApprovalSet.test.ts @@ -0,0 +1,214 @@ +/** + * useRunsPendingApprovalSet — unit tests. + * + * Verifies: no poll when every run is terminal (or the list is empty); polls + * `approval_list_pending` every 3s while a run is `running`; collects only + * flow-origin (`source_context.kind === 'flow'`) matches into the returned + * Set; a chat-origin approval (no flow `source_context`) is excluded; a + * failed poll keeps the last-known set instead of clearing it; teardown once + * every run settles to terminal; cleanup on unmount. + */ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { PendingApproval } from '../../services/api/approvalApi'; +import type { FlowRun } from '../../services/api/flowsApi'; +import { resolveDisplayStatus, useRunsPendingApprovalSet } from '../useRunsPendingApprovalSet'; + +const fetchPendingApprovals = vi.hoisted(() => vi.fn()); +vi.mock('../../services/api/approvalApi', () => ({ fetchPendingApprovals })); + +function makeRun(overrides: Partial = {}): 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, + }; +} + +function makeApproval(overrides: Partial = {}): PendingApproval { + return { + request_id: 'req-1', + tool_name: 'shell', + action_summary: 'Run `shell`', + args_redacted: {}, + session_id: 'session-1', + created_at: '2026-01-01T00:00:00Z', + expires_at: null, + source_context: { kind: 'flow', flow_id: 'flow-1', run_id: 'run-1' }, + ...overrides, + }; +} + +describe('useRunsPendingApprovalSet', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('does not poll when every run is terminal', async () => { + fetchPendingApprovals.mockResolvedValue([]); + renderHook(() => + useRunsPendingApprovalSet([ + makeRun({ status: 'completed' }), + makeRun({ id: 'run-2', status: 'failed' }), + ]) + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); + }); + expect(fetchPendingApprovals).not.toHaveBeenCalled(); + }); + + it('does not poll for an empty runs list', async () => { + fetchPendingApprovals.mockResolvedValue([]); + renderHook(() => useRunsPendingApprovalSet([])); + + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(fetchPendingApprovals).not.toHaveBeenCalled(); + }); + + it('polls every 3s while a run is running and includes it when a matching flow approval exists', async () => { + fetchPendingApprovals.mockResolvedValue([makeApproval({ request_id: 'req-a' })]); + const { result } = renderHook(() => + useRunsPendingApprovalSet([makeRun({ status: 'running' })]) + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(1); + expect(result.current.has('run-1')).toBe(true); + + await act(async () => { + await vi.advanceTimersByTimeAsync(3000); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(2); + }); + + it('excludes a running run with no matching pending approval', async () => { + fetchPendingApprovals.mockResolvedValue([]); + const { result } = renderHook(() => + useRunsPendingApprovalSet([makeRun({ status: 'running' })]) + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.size).toBe(0); + }); + + it('excludes a chat-origin approval (no flow source_context)', async () => { + fetchPendingApprovals.mockResolvedValue([ + makeApproval({ request_id: 'req-chat', source_context: undefined }), + ]); + const { result } = renderHook(() => + useRunsPendingApprovalSet([makeRun({ status: 'running' })]) + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.size).toBe(0); + }); + + it('keeps the last-known set when a poll fails', async () => { + fetchPendingApprovals.mockResolvedValueOnce([makeApproval({ request_id: 'req-a' })]); + const { result, rerender } = renderHook( + ({ runs }: { runs: FlowRun[] }) => useRunsPendingApprovalSet(runs), + { initialProps: { runs: [makeRun({ status: 'running' })] } } + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.has('run-1')).toBe(true); + + fetchPendingApprovals.mockRejectedValueOnce(new Error('network down')); + rerender({ runs: [makeRun({ status: 'running' })] }); + await act(async () => { + await vi.advanceTimersByTimeAsync(3000); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(2); + expect(result.current.has('run-1')).toBe(true); + + // A subsequent successful tick keeps polling on the same cadence. + fetchPendingApprovals.mockResolvedValueOnce([makeApproval({ request_id: 'req-a' })]); + await act(async () => { + await vi.advanceTimersByTimeAsync(3000); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(3); + }); + + it('tears down polling once every run settles to terminal', async () => { + fetchPendingApprovals.mockResolvedValue([makeApproval({ request_id: 'req-a' })]); + const { rerender } = renderHook( + ({ runs }: { runs: FlowRun[] }) => useRunsPendingApprovalSet(runs), + { initialProps: { runs: [makeRun({ status: 'running' })] } } + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(1); + + rerender({ runs: [makeRun({ status: 'completed' })] }); + + fetchPendingApprovals.mockClear(); + await act(async () => { + await vi.advanceTimersByTimeAsync(20_000); + }); + expect(fetchPendingApprovals).not.toHaveBeenCalled(); + }); + + it('cleans up pending timers on unmount', async () => { + fetchPendingApprovals.mockResolvedValue([]); + const { unmount } = renderHook(() => + useRunsPendingApprovalSet([makeRun({ status: 'running' })]) + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(fetchPendingApprovals).toHaveBeenCalledTimes(1); + + unmount(); + + fetchPendingApprovals.mockClear(); + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(fetchPendingApprovals).not.toHaveBeenCalled(); + }); +}); + +describe('resolveDisplayStatus', () => { + it('overrides to pending_approval when running and the run id is in the pending set', () => { + const run = makeRun({ status: 'running' }); + const pendingRunIds = new Set(['run-1']); + expect(resolveDisplayStatus(run, pendingRunIds)).toBe('pending_approval'); + }); + + it('leaves the status untouched when running but not in the pending set', () => { + const run = makeRun({ status: 'running' }); + expect(resolveDisplayStatus(run, new Set())).toBe('running'); + }); + + it('leaves non-running statuses untouched even if the id is (stale) in the pending set', () => { + const run = makeRun({ status: 'completed' }); + const pendingRunIds = new Set(['run-1']); + expect(resolveDisplayStatus(run, pendingRunIds)).toBe('completed'); + }); +}); diff --git a/app/src/hooks/useRunsPendingApprovalSet.ts b/app/src/hooks/useRunsPendingApprovalSet.ts new file mode 100644 index 000000000..0d185a3ae --- /dev/null +++ b/app/src/hooks/useRunsPendingApprovalSet.ts @@ -0,0 +1,118 @@ +/** + * useRunsPendingApprovalSet — cross-references the shared approval queue + * against a runs LIST so surfaces that show many runs at once (sidebar, + * drawer, all-runs page) can distinguish a run merely `running` from one + * that's actually halted at an interactive `ApprovalGate` mid-run. + * + * The core has no separate DB status for "parked at an approval gate" — a + * run stays `status: "running"` in `FlowRun` while it waits; only the shared + * `openhuman.approval_list_pending` queue (see `useFlowPendingApprovals`, + * the single-run analogue used by `FlowRunInspectorDrawer`) knows it's + * parked, via `PendingApproval.source_context = { kind: "flow", run_id }`. + * + * While at least one run in the list is `running`, this hook polls that + * shared queue every {@link POLL_INTERVAL_MS} and returns the Set of + * `run_id`s with a matching flow-origin pending approval. Callers combine + * this with `run.status` via {@link resolveDisplayStatus} to override the + * DISPLAY status only — `run.status` itself, and `useFlowRunsLiveRefresh` + * (which reads the ORIGINAL runs array), are untouched, so `running` stays + * non-terminal there and the list keeps refetching until the run truly + * finishes. + * + * Mirrors the poll-loop + cleanup shape of `useFlowPendingApprovals` + * (cancelled flag + `setTimeout` reschedule + teardown) and the + * gate-on-activity contract of `useFlowRunsLiveRefresh`. Unlike + * `useFlowPendingApprovals`, a failed poll here does NOT surface an error or + * stop polling — this is a best-effort display hint, not a source of truth, + * so a transient failure just keeps showing the last-known set and retries + * on the next tick. + */ +import debug from 'debug'; +import { useEffect, useState } from 'react'; + +import { fetchPendingApprovals } from '../services/api/approvalApi'; +import type { FlowRun, FlowRunStatus } from '../services/api/flowsApi'; + +const log = debug('flows:runs-pending-approval-set'); + +/** How often to re-poll the shared approval queue while any run is `running`. */ +const POLL_INTERVAL_MS = 3000; + +/** + * Poll `openhuman.approval_list_pending` every {@link POLL_INTERVAL_MS}ms + * while `runs` contains at least one `status === 'running'` entry, and + * return the Set of `run_id`s with a flow-origin pending approval + * (`source_context.kind === 'flow'`). Stops polling — but does not clear the + * last-known set — once every run has left `running` (completed, failed, + * cancelled, or already reflected as `pending_approval`). Best-effort: a + * failed poll is logged and simply retried next tick, keeping the + * last-known set rather than surfacing an error to the caller. + */ +export function useRunsPendingApprovalSet(runs: FlowRun[]): Set { + const [pendingRunIds, setPendingRunIds] = useState>(() => new Set()); + + const hasRunning = runs.some(run => run.status === 'running'); + + useEffect(() => { + if (!hasRunning) { + log('approval polling skipped: no-running-runs'); + return; + } + log('approval polling started'); + + let cancelled = false; + let pollHandle: number | undefined; + + const tick = async () => { + if (cancelled) return; + try { + log('approval poll: calling fetchPendingApprovals'); + const all = await fetchPendingApprovals(); + if (cancelled) return; + const next = new Set(); + for (const approval of all) { + if (approval.source_context?.kind === 'flow') { + next.add(approval.source_context.run_id); + } + } + log('approval poll: succeeded total=%d flow-scoped=%d', all.length, next.size); + setPendingRunIds(next); + } catch (err) { + const errorType = err instanceof Error ? err.name : typeof err; + log('approval poll: failed error_type=%s; preserving-last-known-set', errorType); + // Best-effort — leave `pendingRunIds` as-is and retry next tick. + } finally { + if (!cancelled) { + log('approval poll: scheduling retry delay_ms=%d', POLL_INTERVAL_MS); + pollHandle = window.setTimeout(() => void tick(), POLL_INTERVAL_MS); + } + } + }; + + void tick(); + return () => { + cancelled = true; + if (pollHandle !== undefined) window.clearTimeout(pollHandle); + log('approval polling stopped'); + }; + }, [hasRunning]); + + return pendingRunIds; +} + +/** + * Overrides a run's DISPLAY status to `pending_approval` when it's currently + * `running` AND the shared approval queue (via {@link useRunsPendingApprovalSet}) + * has a flow-origin pending approval for it. Does not mutate `run` or touch + * any other status — callers substitute the result at render sites only + * (status dot / accent / label), leaving `run.status` itself (and anything + * keyed on it, like `useFlowRunsLiveRefresh`'s terminal-status check) intact. + */ +export function resolveDisplayStatus(run: FlowRun, pendingRunIds: Set): FlowRunStatus { + if (run.status === 'running' && pendingRunIds.has(run.id)) { + return 'pending_approval'; + } + return run.status; +} + +export default useRunsPendingApprovalSet; diff --git a/app/src/pages/WorkflowRunsPage.test.tsx b/app/src/pages/WorkflowRunsPage.test.tsx index d03d4e66b..99ffd273a 100644 --- a/app/src/pages/WorkflowRunsPage.test.tsx +++ b/app/src/pages/WorkflowRunsPage.test.tsx @@ -17,6 +17,9 @@ vi.mock('../services/api/flowsApi', () => ({ listFlows: (...a: unknown[]) => listFlows(...a), })); +const fetchPendingApprovals = vi.hoisted(() => vi.fn()); +vi.mock('../services/api/approvalApi', () => ({ fetchPendingApprovals })); + // PanelPage + LoadingState pull i18n/redux we don't need — stub to bare markup. vi.mock('../components/layout/PanelPage', () => ({ default: ({ children }: { children: React.ReactNode }) =>
{children}
, @@ -31,6 +34,8 @@ describe('WorkflowRunsPage', () => { navigateMock.mockReset(); listAllFlowRuns.mockReset(); listFlows.mockReset(); + fetchPendingApprovals.mockReset(); + fetchPendingApprovals.mockResolvedValue([]); }); it('renders aggregate runs with their workflow name and status', async () => { @@ -76,6 +81,43 @@ describe('WorkflowRunsPage', () => { await waitFor(() => expect(screen.getByText('rpc down')).toBeInTheDocument()); }); + it('shows "pending approval" for a running run halted at a matching flow approval gate', async () => { + listAllFlowRuns.mockResolvedValue([ + { id: 'r1', flow_id: 'f1', status: 'running', started_at: '2026-01-01T00:00:00Z' }, + ]); + listFlows.mockResolvedValue([{ id: 'f1', name: 'Daily digest' }]); + fetchPendingApprovals.mockResolvedValue([ + { + request_id: 'req-1', + tool_name: 'SLACK_SEND_MESSAGE', + action_summary: 'Send Slack message', + args_redacted: {}, + session_id: 'session-1', + created_at: '2026-01-01T00:00:00Z', + expires_at: null, + source_context: { kind: 'flow', flow_id: 'f1', run_id: 'r1' }, + }, + ]); + + render(); + + const row = await screen.findByTestId('workflow-run-r1'); + await waitFor(() => expect(row).toHaveTextContent('pending approval')); + }); + + it('leaves a running run without a matching flow approval labeled "running"', async () => { + listAllFlowRuns.mockResolvedValue([ + { id: 'r1', flow_id: 'f1', status: 'running', started_at: '2026-01-01T00:00:00Z' }, + ]); + listFlows.mockResolvedValue([{ id: 'f1', name: 'Daily digest' }]); + fetchPendingApprovals.mockResolvedValue([]); + + render(); + + const row = await screen.findByTestId('workflow-run-r1'); + await waitFor(() => expect(row).toHaveTextContent('running')); + }); + describe('live refresh (useFlowRunsLiveRefresh integration)', () => { afterEach(() => { vi.useRealTimers(); diff --git a/app/src/pages/WorkflowRunsPage.tsx b/app/src/pages/WorkflowRunsPage.tsx index 874dd0de3..756985400 100644 --- a/app/src/pages/WorkflowRunsPage.tsx +++ b/app/src/pages/WorkflowRunsPage.tsx @@ -13,6 +13,10 @@ 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 { + resolveDisplayStatus, + useRunsPendingApprovalSet, +} from '../hooks/useRunsPendingApprovalSet'; import { useT } from '../lib/i18n/I18nContext'; import { type Flow, @@ -77,6 +81,7 @@ export default function WorkflowRunsPage() { }, []); useFlowRunsLiveRefresh(runs, refetchRuns); + const pendingRunIds = useRunsPendingApprovalSet(runs); const statusLabel = (status: FlowRunStatus) => t(`flows.allRuns.status.${status}`, status.replace(/_/g, ' ')); @@ -101,31 +106,34 @@ export default function WorkflowRunsPage() {
    - {runs.map(run => ( -
  • - - {run.error && ( -

    - {run.error} -

    - )} -
    • - ))} + {runs.map(run => { + const displayStatus = resolveDisplayStatus(run, pendingRunIds); + return ( +
  • + + {run.error && ( +

    + {run.error} +

    + )} +
    • + ); + })}
)}