diff --git a/app/src/pages/FlowCanvasPage.tsx b/app/src/pages/FlowCanvasPage.tsx index 183990196..7f8c793c3 100644 --- a/app/src/pages/FlowCanvasPage.tsx +++ b/app/src/pages/FlowCanvasPage.tsx @@ -146,6 +146,9 @@ export function asCopilotRepairSeed(state: unknown): CopilotRepairSeed | null { const log = createDebug('app:flows:canvas'); +/** How long the run-error banner stays up before it auto-dismisses. */ +const RUN_ERROR_AUTO_DISMISS_MS = 12_000; + /** Which panel (if any) the canvas side rail shows. Driven by the header toggle. */ type SidePanel = 'copilot' | 'legend' | null; @@ -159,6 +162,28 @@ function errorMessage(err: unknown): string { return err instanceof Error ? err.message : String(err); } +/** + * Run-start failures bubble up through several `thiserror`-wrapped tinyflows + * layers, each re-tagging the one beneath it (e.g. "capability error: graph + * error: capability error: code node exited non-zero (timed_out=false):"). + * The nesting is meaningless to the user — strip repeated " error: " + * wrapper prefixes so only the innermost, actionable tail is shown. Scoped + * to the run-error banner only (`handleRun`'s catch); other error surfaces + * in this file keep the raw message since their shapes differ. Pure and + * exported for direct unit testing without rendering `FlowEditor`. + */ +export function formatRunError(message: string): string { + const wrapperPrefix = /^\w+\s+error:\s*/i; + let rest = message; + let match = wrapperPrefix.exec(rest); + while (match && match[0].length < rest.length) { + rest = rest.slice(match[0].length); + match = wrapperPrefix.exec(rest); + } + const trimmed = rest.replace(/:\s*$/, '').trim(); + return trimmed || message.trim(); +} + /** * True when `title` is "unclaimed" — either blank or exactly the localized * generic placeholder (`t('flows.page.newWorkflow')`, "New workflow") — i.e. @@ -763,6 +788,17 @@ function FlowEditor({ } }, [flowId]); + // Auto-dismiss the run-error banner so a stale failure doesn't linger + // forever over the canvas. Re-runs (and thus restarts the timer) whenever + // `runError` changes — including a new failure replacing an old one, since + // `handleRun` always routes through `setRunError(null)` before setting the + // next message, so a later error is always a distinct effect run. + useEffect(() => { + if (!runError) return; + const timer = setTimeout(() => setRunError(null), RUN_ERROR_AUTO_DISMISS_MS); + return () => clearTimeout(timer); + }, [runError]); + // Return to wherever the user came from rather than always the list. React // Router stamps the initial history entry with key 'default', so when this // page was the first thing loaded (deep link / fresh load) there's nothing to @@ -968,12 +1004,44 @@ function FlowEditor({ /> {runError && ( -
+ // top-14 (not top-3) so this never overlaps the canvas's own + // top-right undo/redo controls, which sit at top-3; max-w-md + // caps how wide a long nested error can grow. When the legend + // palette is open it also docks at top-14/right-3 (w-48), so we + // pull the banner's right edge in past it (right-56) rather than + // centering across the full width, which would let a long + // message reach under the palette and swallow its clicks. +
- {t('flows.editor.runFailed')}: {runError} + className="pointer-events-auto flex w-full max-w-md items-start gap-2 rounded-xl border border-coral-200 bg-coral-50 px-3 py-2 text-xs text-coral-700 dark:border-coral-500/30 dark:bg-coral-500/10 dark:text-coral-300"> + + {t('flows.editor.runFailed')}: {formatRunError(runError)} + +
)} diff --git a/app/src/pages/__tests__/FlowCanvasPage.test.tsx b/app/src/pages/__tests__/FlowCanvasPage.test.tsx index 6ef32d83d..6cbe4a2ca 100644 --- a/app/src/pages/__tests__/FlowCanvasPage.test.tsx +++ b/app/src/pages/__tests__/FlowCanvasPage.test.tsx @@ -15,6 +15,7 @@ import FlowCanvasPage, { asCopilotBuildSeed, asCopilotPrefillSeed, FlowCanvasDraftPage, + formatRunError, isPlaceholderTitle, } from '../FlowCanvasPage'; @@ -23,12 +24,14 @@ const updateFlow = vi.hoisted(() => vi.fn()); const createFlow = vi.hoisted(() => vi.fn()); const validateFlow = vi.hoisted(() => vi.fn()); const listFlowConnections = vi.hoisted(() => vi.fn()); +const runFlow = vi.hoisted(() => vi.fn()); vi.mock('../../services/api/flowsApi', () => ({ getFlow, updateFlow, createFlow, validateFlow, listFlowConnections, + runFlow, })); // Stub the copilot panel: it drives the real chat runtime (redux + socket), @@ -95,6 +98,7 @@ describe('FlowCanvasPage', () => { createFlow.mockReset(); validateFlow.mockReset(); listFlowConnections.mockReset(); + runFlow.mockReset(); validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] }); listFlowConnections.mockResolvedValue([]); updateFlow.mockResolvedValue(makeFlow()); @@ -146,6 +150,93 @@ describe('FlowCanvasPage', () => { expect(screen.getByText('core unreachable')).toBeInTheDocument(); }); + describe('run-error banner (#flows-canvas-error-banner)', () => { + // Run always goes through the header icon → confirm popup → accept, same + // as a real user flow (see `handleRun` wiring around `confirmAction`). + // Two `fireEvent.click`s in a row: the first (Run) synchronously opens + // the confirm popup via a plain `useState` setter, so no flush is needed + // between them. + function clickRun() { + fireEvent.click(screen.getByTestId('flow-canvas-run')); + fireEvent.click(screen.getByTestId('flow-action-confirm-accept')); + } + + it('renders the run-error banner (trimmed) without covering undo/redo, and lets it be dismissed', async () => { + getFlow.mockResolvedValue(makeFlow()); + runFlow.mockRejectedValue( + new Error( + 'capability error: graph error: capability error: code node exited non-zero (timed_out=false):' + ) + ); + renderAtFlowId('test-id'); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + clickRun(); + + const banner = await screen.findByTestId('flow-canvas-run-error'); + // The raw nested "capability error: graph error: capability error: " + // wrapper prefixes are stripped — only the innermost tail remains. + expect(banner).toHaveTextContent('code node exited non-zero (timed_out=false)'); + expect(banner).not.toHaveTextContent('graph error'); + + // Regression for the overlap bug: the banner sits at top-14, well + // below the canvas's own top-3 undo/redo controls, and both remain + // present/interactive rather than one covering the other. + expect(banner.parentElement).toHaveClass('top-14'); + expect(screen.getByTestId('flow-editor-undo')).toBeInTheDocument(); + expect(screen.getByTestId('flow-editor-redo')).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId('flow-canvas-run-error-dismiss')); + expect(screen.queryByTestId('flow-canvas-run-error')).not.toBeInTheDocument(); + }); + + it('auto-dismisses the run-error banner after the timeout, and restarts the timer on a new error', async () => { + getFlow.mockResolvedValue(makeFlow()); + runFlow.mockRejectedValue(new Error('capability error: boom')); + renderAtFlowId('test-id'); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + // Switch to fake timers only now — the load above already went through + // `waitFor` (real timers); the run/timeout portion below only needs + // fake macrotasks plus microtask flushes for the rejected `runFlow` + // promise, matching the FlowRunsDrawer.test.tsx fake-timer precedent. + vi.useFakeTimers(); + try { + clickRun(); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument(); + + // Not yet at the 12s mark — banner is still up. + await act(async () => { + vi.advanceTimersByTime(11_000); + }); + expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument(); + + // A new failure before the old timer fires resets the clock rather + // than stacking on top of it. + clickRun(); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + vi.advanceTimersByTime(11_000); + }); + expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument(); + + await act(async () => { + vi.advanceTimersByTime(2_000); + }); + expect(screen.queryByTestId('flow-canvas-run-error')).not.toBeInTheDocument(); + } finally { + vi.useRealTimers(); + } + }); + }); + it('ignores a stale response for a superseded id after navigating to a new one', async () => { // Deferred promises so the test controls resolution order precisely: the // first (old-id) fetch resolves AFTER the second (new-id) one, mimicking @@ -529,6 +620,28 @@ describe('isPlaceholderTitle', () => { }); }); +describe('formatRunError', () => { + it('strips repeated nested " error: " wrapper prefixes down to the innermost tail', () => { + expect( + formatRunError( + 'capability error: graph error: capability error: code node exited non-zero (timed_out=false):' + ) + ).toBe('code node exited non-zero (timed_out=false)'); + }); + + it('leaves a message with no wrapper prefix unchanged', () => { + expect(formatRunError('flow has no trigger node')).toBe('flow has no trigger node'); + }); + + it('drops a bare trailing colon left over from a single, unstripped wrapper label', () => { + expect(formatRunError('capability error:')).toBe('capability error'); + }); + + it('falls back to the original message rather than returning an empty string', () => { + expect(formatRunError(':')).toBe(':'); + }); +}); + // ----------------------------------------------------------------------------- // Copilot proposal name adoption — accepting a `propose_workflow` proposal // carries a top-level `name` the canvas previously dropped, leaving the flow