diff --git a/app/src/components/flows/FlowRunsSidebar.test.tsx b/app/src/components/flows/FlowRunsSidebar.test.tsx new file mode 100644 index 000000000..2f7bcb725 --- /dev/null +++ b/app/src/components/flows/FlowRunsSidebar.test.tsx @@ -0,0 +1,182 @@ +/** + * FlowRunsSidebar — the flow canvas's projected run-history sidebar. Asserts + * the run list renders, a run row opens the {@link FlowRunInspectorDrawer}, + * and (issue B22) the drawer's "Fix with agent" action navigates to this same + * flow's canvas seeded with a `copilotRepair` state and closes the sidebar's + * own drawer — this sidebar is only ever mounted while the user is ALREADY on + * the failing run's own `/flows/:id` canvas (`FlowCanvasPage` projects it into + * the shell sidebar), so re-navigating to the SAME route with a fresh repair + * seed is the fix (see `FlowCanvasPage.tsx`'s `locationKey`-based copilot + * panel remount, which reacts to exactly this navigation). + */ +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { FlowRun } from '../../services/api/flowsApi'; +import { store } from '../../store'; +import FlowRunsSidebar from './FlowRunsSidebar'; + +const listFlowRuns = vi.hoisted(() => vi.fn()); +vi.mock('../../services/api/flowsApi', () => ({ listFlowRuns })); + +// Capture the props handed to the drawer so "Fix with agent" can be invoked +// directly without standing up the drawer's own run-polling machinery +// (mirrors `FlowApprovalCard.test.tsx`'s stub pattern). +const inspectorDrawerProps = vi.hoisted(() => ({ + current: null as Record | null, +})); +vi.mock('./FlowRunInspectorDrawer', () => ({ + FLOW_RUN_STATUS_ACCENT: { + running: '', + completed: '', + completed_with_warnings: '', + pending_approval: '', + failed: '', + cancelled: '', + }, + FLOW_RUN_STATUS_DOT: { + running: '', + completed: '', + completed_with_warnings: '', + pending_approval: '', + failed: '', + cancelled: '', + }, + FLOW_RUN_STATUS_KEY: { + running: 'flowRuns.status.running', + completed: 'flowRuns.status.completed', + completed_with_warnings: 'flowRuns.status.completed_with_warnings', + pending_approval: 'flowRuns.status.pending_approval', + failed: 'flowRuns.status.failed', + cancelled: 'flowRuns.status.cancelled', + }, + FlowRunInspectorDrawer: (props: Record) => { + inspectorDrawerProps.current = props; + return props.runId ? ( +
{props.runId as string}
+ ) : null; + }, +})); + +function makeRun(overrides: Partial = {}): FlowRun { + return { + id: 'run-1', + flow_id: 'flow-1', + thread_id: 'run-1', + status: 'failed', + started_at: '2026-07-13T18:23:00Z', + finished_at: '2026-07-13T18:23:05Z', + steps: [], + pending_approvals: [], + error: 'GMAIL_SEND_EMAIL: empty body', + ...overrides, + }; +} + +/** Renders whatever `location.state` a navigation landed with, for assertions. */ +function LocationStateProbe() { + const location = useLocation(); + return
{JSON.stringify(location.state)}
; +} + +function renderSidebar(flowId = 'flow-1') { + return render( + + + + } /> + + + + ); +} + +describe('FlowRunsSidebar', () => { + beforeEach(() => { + vi.clearAllMocks(); + inspectorDrawerProps.current = null; + }); + + it('lists runs and opens the inspector drawer for the clicked run', async () => { + listFlowRuns.mockResolvedValue([makeRun()]); + renderSidebar(); + + const row = await screen.findByTestId('flow-runs-sidebar-run-run-1'); + expect(screen.queryByTestId('flow-run-inspector-drawer-stub')).not.toBeInTheDocument(); + + fireEvent.click(row); + + expect(screen.getByTestId('flow-run-inspector-drawer-stub')).toHaveTextContent('run-1'); + }); + + it('passes onFixWithAgent through to the run inspector drawer', async () => { + listFlowRuns.mockResolvedValue([makeRun()]); + renderSidebar(); + + fireEvent.click(await screen.findByTestId('flow-runs-sidebar-run-run-1')); + + expect(inspectorDrawerProps.current?.onFixWithAgent).toBeInstanceOf(Function); + }); + + it('"Fix with agent" closes the drawer and navigates to the same flow seeded with the repair context (B22)', async () => { + listFlowRuns.mockResolvedValue([makeRun()]); + render( + + + + + + + + } + /> + + + + ); + + fireEvent.click(await screen.findByTestId('flow-runs-sidebar-run-run-1')); + expect(screen.getByTestId('flow-run-inspector-drawer-stub')).toBeInTheDocument(); + + act(() => { + ( + inspectorDrawerProps.current?.onFixWithAgent as (request: { + flowId: string; + runId: string; + error?: string | null; + failingNodeIds?: string[]; + }) => void + )({ + flowId: 'flow-1', + runId: 'run-1', + error: 'GMAIL_SEND_EMAIL: empty body', + failingNodeIds: ['send_summary'], + }); + }); + + // The sidebar's own run-inspector drawer closes (repair takes over). + await waitFor(() => + expect(screen.queryByTestId('flow-run-inspector-drawer-stub')).not.toBeInTheDocument() + ); + const probe = screen.getByTestId('location-state-probe'); + expect(JSON.parse(probe.textContent ?? 'null')).toEqual({ + copilotRepair: { + runId: 'run-1', + error: 'GMAIL_SEND_EMAIL: empty body', + failingNodeIds: ['send_summary'], + }, + }); + }); + + it('shows the empty state when there are no runs', async () => { + listFlowRuns.mockResolvedValue([]); + renderSidebar(); + + expect(await screen.findByTestId('flow-runs-sidebar-empty')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/flows/FlowRunsSidebar.tsx b/app/src/components/flows/FlowRunsSidebar.tsx index 83d474f42..42edb95c9 100644 --- a/app/src/components/flows/FlowRunsSidebar.tsx +++ b/app/src/components/flows/FlowRunsSidebar.tsx @@ -11,6 +11,7 @@ */ import createDebug from 'debug'; import { useCallback, useEffect, useState } from 'react'; +import { useNavigate } from 'react-router-dom'; import { useT } from '../../lib/i18n/I18nContext'; import { type FlowRun, listFlowRuns } from '../../services/api/flowsApi'; @@ -19,6 +20,7 @@ import { FLOW_RUN_STATUS_ACCENT, FLOW_RUN_STATUS_DOT, FLOW_RUN_STATUS_KEY, + type FlowRepairRequest, FlowRunInspectorDrawer, } from './FlowRunInspectorDrawer'; @@ -44,11 +46,38 @@ export interface FlowRunsSidebarProps { export default function FlowRunsSidebar({ flowId }: FlowRunsSidebarProps) { const { t } = useT(); + const navigate = useNavigate(); const [runs, setRuns] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [selectedRunId, setSelectedRunId] = useState(null); + // "Fix with agent" (issue B22) — this sidebar is only ever mounted while + // already on the failed run's own `/flows/:id` canvas (`FlowCanvasPage` + // projects it into the shell sidebar), so re-navigating to the SAME route + // with a fresh `copilotRepair` state is enough to open the canvas copilot + // preloaded with the failure — same mechanism `FlowsPage`'s run-history + // drawer uses to reach this page from elsewhere. `replace: true` avoids + // stacking a new history entry per click on top of the page the user is + // already viewing. + const handleFixWithAgent = useCallback( + (request: FlowRepairRequest) => { + log('fix with agent: flow=%s run=%s', request.flowId, request.runId); + setSelectedRunId(null); + navigate(`/flows/${request.flowId}`, { + replace: true, + state: { + copilotRepair: { + runId: request.runId, + error: request.error, + failingNodeIds: request.failingNodeIds, + }, + }, + }); + }, + [navigate] + ); + const load = useCallback(async () => { log('loading runs for flow=%s', flowId); setLoading(true); @@ -145,7 +174,11 @@ export default function FlowRunsSidebar({ flowId }: FlowRunsSidebarProps) { - setSelectedRunId(null)} /> + setSelectedRunId(null)} + onFixWithAgent={handleFixWithAgent} + /> ); } diff --git a/app/src/components/notifications/FlowApprovalCard.test.tsx b/app/src/components/notifications/FlowApprovalCard.test.tsx index eacc8c0b0..81787fc1a 100644 --- a/app/src/components/notifications/FlowApprovalCard.test.tsx +++ b/app/src/components/notifications/FlowApprovalCard.test.tsx @@ -1,15 +1,20 @@ /** - * Approve/Dismiss/View-run contract for the flow-pending-approval - * notification card (issues B3a + B3b). Asserts that Approve reads - * `{ flow_id, thread_id, node_ids }` from the notification's action payload, - * calls `flowsApi.resumeFlow` with those args, clears the notification on - * success, surfaces a localized error on failure (including when `node_ids` - * contains non-string entries — an invalid payload), that Dismiss clears the - * notification WITHOUT calling any RPC (there is no `flows_deny` endpoint - * yet), and that "View run" opens the {@link FlowRunInspectorDrawer}. + * Approve/Dismiss/View-run/Fix-with-agent contract for the + * flow-pending-approval notification card (issues B3a + B3b + B22). Asserts + * that Approve reads `{ flow_id, thread_id, node_ids }` from the notification's + * action payload, calls `flowsApi.resumeFlow` with those args, clears the + * notification on success, surfaces a localized error on failure (including + * when `node_ids` contains non-string entries — an invalid payload), that + * Dismiss clears the notification WITHOUT calling any RPC (there is no + * `flows_deny` endpoint yet), that "View run" opens the + * {@link FlowRunInspectorDrawer}, and that the drawer's "Fix with agent" + * action navigates to the flow's canvas seeded with a `copilotRepair` state + * (issue B22 — this card can render anywhere in the app, so it always + * navigates rather than assuming the canvas is already open). */ -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 { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { store } from '../../store'; @@ -19,9 +24,20 @@ import FlowApprovalCard from './FlowApprovalCard'; const resumeFlow = vi.hoisted(() => vi.fn()); vi.mock('../../services/api/flowsApi', () => ({ resumeFlow })); +// Capture the props `FlowApprovalCard` hands the drawer (mirrors +// `FlowCanvasPage.test.tsx`'s copilot-panel stub pattern) so tests can invoke +// `onFixWithAgent` directly without needing the drawer's own run-polling +// machinery. +const inspectorDrawerProps = vi.hoisted(() => ({ + current: null as Record | null, +})); vi.mock('../flows/FlowRunInspectorDrawer', () => ({ - FlowRunInspectorDrawer: ({ runId }: { runId: string | null; onClose: () => void }) => - runId ?
{runId}
: null, + FlowRunInspectorDrawer: (props: Record) => { + inspectorDrawerProps.current = props; + return props.runId ? ( +
{props.runId as string}
+ ) : null; + }, })); function makeItem(overrides: Partial = {}): NotificationItem { @@ -43,10 +59,21 @@ function makeItem(overrides: Partial = {}): NotificationItem { }; } +/** Renders whatever `location.state` a navigation landed with, for assertions. */ +function LocationStateProbe() { + const location = useLocation(); + return
{JSON.stringify(location.state)}
; +} + function renderCard(item: NotificationItem) { return render( - + + + } /> + } /> + + ); } @@ -55,6 +82,7 @@ describe('FlowApprovalCard', () => { beforeEach(() => { vi.clearAllMocks(); store.dispatch({ type: 'notifications/clearAll' }); + inspectorDrawerProps.current = null; }); it('renders both Approve and Dismiss buttons', () => { @@ -227,4 +255,46 @@ describe('FlowApprovalCard', () => { expect(drawer).toBeInTheDocument(); expect(drawer).toHaveTextContent('thread-1'); }); + + it('passes onFixWithAgent through to the run inspector drawer', () => { + renderCard(makeItem()); + fireEvent.click(screen.getByTestId('flow-approval-view-run')); + + expect(inspectorDrawerProps.current?.onFixWithAgent).toBeInstanceOf(Function); + }); + + it('"Fix with agent" navigates to the flow canvas seeded with the repair context (B22)', async () => { + renderCard(makeItem()); + fireEvent.click(screen.getByTestId('flow-approval-view-run')); + expect(screen.getByTestId('flow-run-inspector-drawer-stub')).toBeInTheDocument(); + + act(() => { + ( + inspectorDrawerProps.current?.onFixWithAgent as (request: { + flowId: string; + runId: string; + error?: string | null; + failingNodeIds?: string[]; + }) => void + )({ + flowId: 'flow-1', + runId: 'thread-1', + error: 'GMAIL_SEND_EMAIL: empty body', + failingNodeIds: ['send_summary'], + }); + }); + + // Navigated away — the drawer stub (and the whole card) unmounts. + await waitFor(() => + expect(screen.queryByTestId('flow-run-inspector-drawer-stub')).not.toBeInTheDocument() + ); + const probe = await screen.findByTestId('location-state-probe'); + expect(JSON.parse(probe.textContent ?? 'null')).toEqual({ + copilotRepair: { + runId: 'thread-1', + error: 'GMAIL_SEND_EMAIL: empty body', + failingNodeIds: ['send_summary'], + }, + }); + }); }); diff --git a/app/src/components/notifications/FlowApprovalCard.tsx b/app/src/components/notifications/FlowApprovalCard.tsx index cb4fb7f84..8897e7dcb 100644 --- a/app/src/components/notifications/FlowApprovalCard.tsx +++ b/app/src/components/notifications/FlowApprovalCard.tsx @@ -28,6 +28,7 @@ */ import debug from 'debug'; import { useState } from 'react'; +import { useNavigate } from 'react-router-dom'; import { useT } from '../../lib/i18n/I18nContext'; import { resumeFlow } from '../../services/api/flowsApi'; @@ -37,7 +38,7 @@ import { markRead, type NotificationItem, } from '../../store/notificationSlice'; -import { FlowRunInspectorDrawer } from '../flows/FlowRunInspectorDrawer'; +import { type FlowRepairRequest, FlowRunInspectorDrawer } from '../flows/FlowRunInspectorDrawer'; import Button from '../ui/Button'; const log = debug('notifications:flow-approval-card'); @@ -71,10 +72,31 @@ interface Props { const FlowApprovalCard = ({ notification: n }: Props) => { const { t } = useT(); const dispatch = useAppDispatch(); + const navigate = useNavigate(); const [pending, setPending] = useState<'approve' | null>(null); const [error, setError] = useState(null); const [inspecting, setInspecting] = useState(false); + // "Fix with agent" (issue B22) — a run opened from this notification can + // fail with the gate never actually reached (or after it), so the same + // repair action `FlowsPage`'s run-history drawer offers must be reachable + // from here too. This card can render from anywhere in the app (it's a + // `CoreNotification` surface), so — unlike `FlowRunsSidebar`, which is + // always already on the flow's own canvas — always navigate there first. + const handleFixWithAgent = (request: FlowRepairRequest) => { + log('fix with agent: notification=%s flow=%s run=%s', n.id, request.flowId, request.runId); + setInspecting(false); + navigate(`/flows/${request.flowId}`, { + state: { + copilotRepair: { + runId: request.runId, + error: request.error, + failingNodeIds: request.failingNodeIds, + }, + }, + }); + }; + const payload = n.actions?.[0]?.payload; const parsed = isFlowApprovalPayload(payload) ? payload : null; @@ -204,6 +226,7 @@ const FlowApprovalCard = ({ notification: n }: Props) => { setInspecting(false)} + onFixWithAgent={handleFixWithAgent} /> )} diff --git a/app/src/pages/FlowCanvasPage.tsx b/app/src/pages/FlowCanvasPage.tsx index 0752378ef..868ee6fef 100644 --- a/app/src/pages/FlowCanvasPage.tsx +++ b/app/src/pages/FlowCanvasPage.tsx @@ -138,6 +138,7 @@ function FlowEditor({ initialCopilotSeed = null, initialBuildSeed = null, onBuildSeedConsumed, + locationKey, }: { editorFlow: EditorFlow; initialCopilotSeed?: CopilotRepairSeed | null; @@ -145,6 +146,16 @@ function FlowEditor({ initialBuildSeed?: CopilotBuildSeed | null; /** Clear the route's build seed once the copilot has dispatched it (#4597). */ onBuildSeedConsumed?: () => void; + /** + * The route's `location.key` (issue B22) — react-router mints a fresh one on + * every navigation, including a same-path repeat navigation (e.g. the "Fix + * with agent" action from {@link FlowRunsSidebar}, which stays on this same + * `/flows/:id` route and so does NOT remount `FlowEditor`). Folded into the + * copilot panel's `key` below so a repair seed arriving without a page-level + * remount still forces a fresh panel mount — required for the panel's + * once-per-mount auto-fire guard (`repairSentRef`) to actually fire again. + */ + locationKey: string; }) { const { t } = useT(); const navigate = useNavigate(); @@ -206,6 +217,19 @@ function FlowEditor({ const [copilotOpen, setCopilotOpen] = useState( initialCopilotSeed !== null || initialBuildSeed !== null ); + // Issue B22: a repair seed can also arrive WITHOUT a `FlowEditor` remount — + // "Fix with agent" clicked from `FlowRunsSidebar` stays on this same + // `/flows/:id` route (only `location.state`/`location.key` change), so the + // `useState` initializer above (mount-only) won't re-open the panel for it. + // Re-assert open whenever a new repair seed prop arrives — done during + // render (React's "adjusting state when a prop changes" pattern) rather + // than a `useEffect`, so it lands in the same render pass instead of an + // extra one. + const [seenCopilotSeed, setSeenCopilotSeed] = useState(initialCopilotSeed); + if (initialCopilotSeed !== seenCopilotSeed) { + setSeenCopilotSeed(initialCopilotSeed); + if (initialCopilotSeed) setCopilotOpen(true); + } // Per-workflow copilot thread: seeded from the session cache so opening/closing // the panel (or switching flows and back) resumes the same conversation // instead of starting a fresh `workflow_builder` thread each time. @@ -323,6 +347,14 @@ function FlowEditor({ // the new flow's canonical `/flows/:id` canvas so further saves update it. // Rejections propagate so the canvas surfaces the failure inline (and leaves // the draft dirty). + // + // Issue B21: `flows_update` re-validates/normalizes the graph server-side + // (schema migration, id defaults, port normalization, etc.) before + // persisting, so the canonical saved shape can differ from what the client + // sent. Re-sync the canvas draft from the RESPONSE (not just the just-sent + // `next`) and bump `canvasVersion` so the editable canvas re-seeds from the + // canonical persisted graph immediately — matching what a navigate-away- + // and-back remount would show, without requiring one. const handleSave = useCallback( async (next: WorkflowGraph) => { if (isDraft) { @@ -338,9 +370,17 @@ function FlowEditor({ return; } log('save: flow id=%s nodes=%d edges=%d', flowId, next.nodes.length, next.edges.length); - await updateFlow(flowId, { graph: next }); - persistedGraphRef.current = next; - log('save: flow id=%s persisted', flowId); + const updated = await updateFlow(flowId, { graph: next }); + const persisted = updated.graph as WorkflowGraph; + persistedGraphRef.current = persisted; + setDraftGraph(persisted); + setCanvasVersion(v => v + 1); + log( + 'save: flow id=%s persisted — canvas re-synced from response nodes=%d edges=%d', + flowId, + persisted.nodes.length, + persisted.edges.length + ); }, [isDraft, flowId, name, requireApproval, navigate] ); @@ -534,6 +574,14 @@ function FlowEditor({ {copilotOpen && ( ); } @@ -735,6 +784,7 @@ export function FlowCanvasDraftPage() { graph: draft.graph, requireApproval: draft.requireApproval, }} + locationKey={location.key} /> diff --git a/app/src/pages/__tests__/FlowCanvasPage.test.tsx b/app/src/pages/__tests__/FlowCanvasPage.test.tsx index bb157b286..27defdee4 100644 --- a/app/src/pages/__tests__/FlowCanvasPage.test.tsx +++ b/app/src/pages/__tests__/FlowCanvasPage.test.tsx @@ -194,6 +194,78 @@ describe('FlowCanvasPage', () => { ]); }); + // Issue B21: `flows_update` re-validates/normalizes the graph server-side + // before persisting, so the canonical response can legitimately differ from + // what the client sent (schema migration, id defaults, etc.). Previously the + // canvas re-baselined against its OWN pre-save nodes/edges and ignored the + // response entirely — the canonical shape only ever appeared after a + // navigate-away-and-back remount refetched it via `flows_get`. Assert the + // canvas now reflects the SAVE RESPONSE's graph immediately, with no + // navigation and no remount of `FlowCanvasPage`. + it('re-syncs the canvas from the flows_update response on save, without a remount (B21)', async () => { + getFlow.mockResolvedValue(makeFlow()); + // The server "normalizes" the saved graph: it accepts the client's + // trigger+agent nodes but also injects a third node the client never + // added (standing in for a server-side migration/default-fill), and + // renames the trigger. A stale canvas would keep showing only the two + // client-added nodes named "Start"/"New agent". + updateFlow.mockResolvedValue( + makeFlow({ + graph: { + schema_version: 1, + id: 'test-id', + name: 'Daily digest', + nodes: [ + { + id: 't', + kind: 'trigger', + name: 'Start (normalized)', + config: {}, + ports: [], + position: { x: 0, y: 0 }, + }, + { + id: 'new-agent-0', + kind: 'agent', + name: 'New agent', + config: {}, + ports: [], + position: { x: 80, y: 80 }, + }, + { + id: 'server-added', + kind: 'transform', + name: 'Server-added node', + config: {}, + ports: [], + position: { x: 160, y: 160 }, + }, + ], + edges: [], + }, + }) + ); + renderEditor(); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + fireEvent.click(screen.getByTestId('flow-palette-item-agent')); + expect(screen.getAllByTestId('flow-node')).toHaveLength(2); + + fireEvent.click(screen.getByTestId('flow-editor-save')); + await waitFor(() => expect(updateFlow).toHaveBeenCalledTimes(1)); + + // The canvas now shows the RESPONSE's three nodes (including the one the + // client never added and the renamed trigger) — no navigation, no + // `flows_get` refetch, no remount required. + await waitFor(() => expect(screen.getAllByTestId('flow-node')).toHaveLength(3)); + expect(screen.getByText('Start (normalized)')).toBeInTheDocument(); + expect(screen.getByText('Server-added node')).toBeInTheDocument(); + // Still the same page/component — proving this wasn't a navigate-away + // remount refetch in disguise. + expect(getFlow).toHaveBeenCalledTimes(1); + expect(screen.getByTestId('flow-canvas-page')).toBeInTheDocument(); + }); + it('does not prompt when navigating Back with no unsaved changes', async () => { getFlow.mockResolvedValue(makeFlow()); renderEditor();