From 7c7b22e511943c58ddd0f9282ff84c41a9509b14 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 7 Jul 2026 23:35:05 -0700 Subject: [PATCH] FN-7652: fix live-run status falsely showing Run failed for successful automations Reconcile the automation live-run panel's terminal status with the authoritative run result so successful runs no longer flash "Run failed". - ScheduledTasksModal/RoutineCard now reconcile SSE terminal status against the POST/registry result instead of trusting raw stream teardown. - Gate benign SSE teardown (post-terminal close, reconnect exhaustion) from being surfaced as a failure state. - Apply the same reconciliation to both /routines/:id/run/stream and /automations/:id/run/stream routes in routes.ts. - Add regression coverage in RoutineCard, ScheduledTasksModal, and routes-automation tests. - Add a patch changeset documenting the fix. Files changed: .changeset/fn-7652-automation-live-run-false-failure.md | 7 ++ packages/dashboard/app/components/ScheduledTasksModal.tsx | 66 ++++++++++++++-- packages/dashboard/app/components/__tests__/RoutineCard.test.tsx | 37 +++++++++ packages/dashboard/app/components/__tests__/ScheduledTasksModal.test.tsx | 78 +++++++++++++++++++ packages/dashboard/src/__tests__/routes-automation.test.ts | 87 ++++++++++++++++++++++ packages/dashboard/src/routes.ts | 52 ++++++++++++- 6 files changed, 319 insertions(+), 8 deletions(-) Fusion-Task-Id: FN-7652 Fusion-Task-Lineage: 61fc89f5-d25d-44de-9699-bc8ad2a0dea6 Co-authored-by: Fusion (runfusion.ai) --- ...-7652-automation-live-run-false-failure.md | 7 ++ .../app/components/ScheduledTasksModal.tsx | 66 ++++++++++++-- .../components/__tests__/RoutineCard.test.tsx | 37 ++++++++ .../__tests__/ScheduledTasksModal.test.tsx | 78 +++++++++++++++++ .../src/__tests__/routes-automation.test.ts | 87 +++++++++++++++++++ packages/dashboard/src/routes.ts | 52 ++++++++++- 6 files changed, 319 insertions(+), 8 deletions(-) create mode 100644 .changeset/fn-7652-automation-live-run-false-failure.md diff --git a/.changeset/fn-7652-automation-live-run-false-failure.md b/.changeset/fn-7652-automation-live-run-false-failure.md new file mode 100644 index 0000000000..c34a4ab108 --- /dev/null +++ b/.changeset/fn-7652-automation-live-run-false-failure.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Automation live output no longer shows "Run failed" for runs that actually succeed. +category: fix +dev: Reconciles the live-run panel terminal status (ScheduledTasksModal/RoutineCard) to the authoritative POST/registry result and gates benign SSE teardown (post-terminal close, reconnect exhaustion) from being surfaced as a failure across both `/routines/:id/run/stream` and `/automations/:id/run/stream`. diff --git a/packages/dashboard/app/components/ScheduledTasksModal.tsx b/packages/dashboard/app/components/ScheduledTasksModal.tsx index 6162b52fd3..f1423a9610 100644 --- a/packages/dashboard/app/components/ScheduledTasksModal.tsx +++ b/packages/dashboard/app/components/ScheduledTasksModal.tsx @@ -143,6 +143,49 @@ export function ScheduledTasksModal({ onClose, addToast, projectId, presentation }); }, []); + /* + FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): + The live-output panel's terminal status must be a function of the AUTHORITATIVE run result (the POST + trigger's `result.success`, mirrored server-side by `AutomationLiveRunRegistry.complete()`), never of + transient SSE stream mechanics. Two benign-teardown paths used to get misread as "the run failed": + (a) `streamRoutineRun` is opened WITHOUT a runId (it races the POST so a slow run still streams live), + so its resilient EventSource can exhaust reconnect attempts and fire `onFatalError("Connection + lost")` for reasons that have nothing to do with the run's outcome (dev-server hiccup, a normal + post-terminal `res.end()` racing the client, etc.) — this must never itself paint "Run failed". + (b) a runId-less stream can (server-permitting) attach to a stale prior run and replay ITS terminal + event; `routes.ts`'s `getForAutoAttach` bounds that window, but the client still treats the + awaited POST result as the single source of truth rather than trusting whichever event happened + to arrive on the SSE channel. + `reconcileLiveRunResult` is the one place that commits a terminal status/line; it is idempotent (a + no-op when the panel already reflects the same terminal status) and drops any output the panel + accumulated after a wrongly-terminal state so no false "Run failed" line can linger once the true + outcome is known. + */ + const reconcileLiveRunResult = useCallback((routineId: string, success: boolean, errorMessage?: string) => { + setLiveRunOutput((previous) => { + const current = previous[routineId]; + const targetStatus: "complete" | "error" = success ? "complete" : "error"; + if (current?.status === targetStatus) { + // Already reflects the authoritative outcome (a genuine SSE terminal event already matched it). + return previous; + } + const line = success + ? t("schedule.liveRunComplete", "Run complete") + : (errorMessage || t("schedule.liveRunError", "Run failed")); + // Only carry forward the transcript if the panel is still mid-run; a mismatched prior terminal + // status (stale-attach or benign-teardown artifact) must not leave its line behind once the real + // outcome supersedes it. + const base = current && current.status === "running" ? current.output : ""; + return { + ...previous, + [routineId]: { + output: base ? `${base}\n${line}` : line, + status: targetStatus, + }, + }; + }); + }, [t]); + /* FNXC:AutomationLiveOutput 2026-06-26-00:00: The modal and embedded Automations view both render RoutineCard, so the run handler owns one SSE stream per routine and passes the accumulated live transcript down instead of duplicating stream logic per presentation. @@ -161,17 +204,17 @@ export function ScheduledTasksModal({ onClose, addToast, projectId, presentation return; } if (event.type === "complete") { - appendLiveRunLine(routineId, t("schedule.liveRunComplete", "Run complete"), "complete"); + reconcileLiveRunResult(routineId, true); liveRunStreamsRef.current[routineId]?.close(); delete liveRunStreamsRef.current[routineId]; return; } if (event.type === "error") { - appendLiveRunLine(routineId, event.message ?? t("schedule.liveRunError", "Run failed"), "error"); + reconcileLiveRunResult(routineId, false, event.message); liveRunStreamsRef.current[routineId]?.close(); delete liveRunStreamsRef.current[routineId]; } - }, [appendLiveRunLine, t]); + }, [appendLiveRunLine, reconcileLiveRunResult]); // ── Routine CRUD handlers ─────────────────────────────────────────────── @@ -233,7 +276,12 @@ export function ScheduledTasksModal({ onClose, addToast, projectId, presentation liveRunStreamsRef.current[routine.id]?.close(); liveRunStreamsRef.current[routine.id] = streamRoutineRun(routine.id, { onEvent: (event) => handleLiveRunEvent(routine.id, event), - onFatalError: (message) => appendLiveRunLine(routine.id, message, "error"), + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): benign SSE teardown (reconnect + // exhaustion, a normal post-terminal close racing the client) is NOT a run-failure signal — + // intentionally a no-op here. The awaited `runRoutine` result below is the sole authority that + // reconciles `liveRunOutput` to the real terminal state; letting this callback paint + // "Run failed" is exactly the FN-7652 bug (a successful run ending in an error-styled panel). + onFatalError: () => {}, }, scopeOptions); try { const { result } = await runRoutine(routine.id, scopeOptions); @@ -245,6 +293,7 @@ export function ScheduledTasksModal({ onClose, addToast, projectId, presentation success: result.success, }, })); + reconcileLiveRunResult(routine.id, result.success, result.error); if (result.success) { addToast(t("schedule.routineSuccess", "\"{{name}}\" completed successfully", { name: routine.name }), "success"); } else { @@ -252,14 +301,19 @@ export function ScheduledTasksModal({ onClose, addToast, projectId, presentation } await loadRoutines(); } catch (err) { - addToast(getErrorMessage(err) || t("schedule.runError", "Failed to run routine"), "error"); + const message = getErrorMessage(err) || t("schedule.runError", "Failed to run routine"); + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): the POST itself never resolved with a + // result, so this IS a genuine failure — reconcile the live panel to the real error too, not + // just the toast, so it doesn't linger stuck on "running". + reconcileLiveRunResult(routine.id, false, message); + addToast(message, "error"); } finally { liveRunStreamsRef.current[routine.id]?.close(); delete liveRunStreamsRef.current[routine.id]; setRunningRoutineId(null); } }, - [addToast, appendLiveRunLine, handleLiveRunEvent, loadRoutines, scopeOptions, t], + [addToast, handleLiveRunEvent, loadRoutines, reconcileLiveRunResult, scopeOptions, t], ); const handleToggleRoutine = useCallback( diff --git a/packages/dashboard/app/components/__tests__/RoutineCard.test.tsx b/packages/dashboard/app/components/__tests__/RoutineCard.test.tsx index a5158eefe6..85222aeb14 100644 --- a/packages/dashboard/app/components/__tests__/RoutineCard.test.tsx +++ b/packages/dashboard/app/components/__tests__/RoutineCard.test.tsx @@ -209,6 +209,43 @@ describe("RoutineCard", () => { expect(screen.getByText("done line")).toBeDefined(); }); + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): the live-output panel's status class is + // the visible contract for the terminal state ScheduledTasksModal reconciles from the authoritative + // run result — assert `complete` never carries the `error` class (and vice versa) so a successful + // run can never render with error-styled chrome. + it("renders the complete status class (never error) for a reconciled successful run", () => { + const { container } = render( + , + ); + const panel = container.querySelector(".routine-live-output"); + expect(panel?.className).toContain("complete"); + expect(panel?.className).not.toContain("error"); + expect(screen.queryByText(/Run failed/)).toBeNull(); + }); + + it("renders the error status class for a genuinely failed run", () => { + const { container } = render( + , + ); + const panel = container.querySelector(".routine-live-output"); + expect(panel?.className).toContain("error"); + expect(screen.getByText("Run failed")).toBeDefined(); + }); + it("does not render a live-output panel when liveRunOutput is null", () => { render( { }); }); + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): regression coverage for the false + // "Run failed" bug — the live-output terminal status must be driven by the authoritative POST + // result, and benign SSE teardown (reconnect exhaustion → onFatalError) must never itself render an + // error state for a run whose real result is success. + it("reconciles live output to complete (never Run failed) when a success run's stream benignly errors out", async () => { + const routine = makeRoutine({ name: "My Routine" }); + mockFetchRoutines.mockResolvedValue([routine]); + let streamHandlers: { onEvent: (event: any) => void; onFatalError?: (message: string) => void } | undefined; + mockStreamRoutineRun.mockImplementation((_id, handlers) => { + streamHandlers = handlers; + return { close: vi.fn() }; + }); + mockRunRoutine.mockImplementation(async () => { + // Simulate the resilient EventSource exhausting reconnect attempts (a normal post-terminal + // teardown / transient connection blip) firing BEFORE the POST result resolves. + streamHandlers?.onFatalError?.("Connection lost"); + return { + result: { + routineId: routine.id, + success: true, + output: "Done", + startedAt: "2026-04-08T00:00:00.000Z", + completedAt: "2026-04-08T00:01:00.000Z", + }, + }; + }); + + render(); + + await waitFor(() => { + expect(screen.getByText("My Routine")).toBeDefined(); + }); + fireEvent.click(screen.getByLabelText("Run My Routine now")); + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith('"My Routine" completed successfully', "success"); + }); + + const panel = document.querySelector(".routine-live-output"); + expect(panel).not.toBeNull(); + expect(panel?.className).toContain("complete"); + expect(panel?.className).not.toContain("error"); + expect(panel?.textContent ?? "").not.toMatch(/Run failed/); + expect(screen.getByText(/Run complete/)).toBeDefined(); + }); + + it("still renders Run failed with the real error message for a genuinely failed run", async () => { + const routine = makeRoutine({ name: "My Routine" }); + mockFetchRoutines.mockResolvedValue([routine]); + mockStreamRoutineRun.mockReturnValue({ close: vi.fn() }); + mockRunRoutine.mockResolvedValue({ + result: { + routineId: routine.id, + success: false, + output: "", + error: "backup command exited 1", + startedAt: "2026-04-08T00:00:00.000Z", + completedAt: "2026-04-08T00:01:00.000Z", + }, + }); + + render(); + + await waitFor(() => { + expect(screen.getByText("My Routine")).toBeDefined(); + }); + fireEvent.click(screen.getByLabelText("Run My Routine now")); + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith('"My Routine" failed: backup command exited 1', "error"); + }); + + const panel = document.querySelector(".routine-live-output"); + expect(panel).not.toBeNull(); + expect(panel?.className).toContain("error"); + expect(screen.getAllByText(/backup command exited 1/).length).toBeGreaterThan(0); + }); + it("deletes routines after confirmation", async () => { const routine = makeRoutine({ name: "My Routine" }); mockFetchRoutines.mockResolvedValue([routine]); diff --git a/packages/dashboard/src/__tests__/routes-automation.test.ts b/packages/dashboard/src/__tests__/routes-automation.test.ts index a5164fcbe5..fcf7766a70 100644 --- a/packages/dashboard/src/__tests__/routes-automation.test.ts +++ b/packages/dashboard/src/__tests__/routes-automation.test.ts @@ -945,6 +945,43 @@ describe("Automation routes", () => { expect(String(streamRes.body)).toContain("Live run not found or expired"); }); + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): the dashboard's manual "Run" trigger opens + // this stream WITHOUT a runId (it races the POST). A runId-less stream that attaches after the run + // already succeeded must deliver `complete`, never a terminal `error` — this is the server-side half + // of the false "Run failed"-for-a-success-run regression. + it("delivers complete, never a terminal error, for a runId-less stream attached after a successful run", async () => { + const mockStore = createMockAutomationStore(); + mockStore.getSchedule.mockResolvedValue({ ...FAKE_SCHEDULE, command: "echo manual-run-ok" }); + const { app } = buildApp(mockStore); + + const runRes = await REQUEST(app, "POST", "/api/automations/sched-001/run"); + expect(runRes.status).toBe(200); + expect(runRes.body.result.success).toBe(true); + + const streamRes = await performRequest(app, "GET", "/api/automations/sched-001/run/stream"); + expect(streamRes.status).toBe(200); + const body = String(streamRes.body); + expect(body).toContain("event: complete"); + expect(body).not.toContain("event: error"); + }); + + it("still delivers a real terminal error event for a runId-less stream after a genuinely failed run", async () => { + const mockStore = createMockAutomationStore(); + mockStore.getSchedule.mockResolvedValue({ ...FAKE_SCHEDULE, command: "exit 1" }); + const { app } = buildApp(mockStore); + + const runRes = await REQUEST(app, "POST", "/api/automations/sched-001/run"); + expect(runRes.status).toBe(200); + expect(runRes.body.result.success).toBe(false); + expect(runRes.body.result.error).toBeTruthy(); + + const streamRes = await performRequest(app, "GET", "/api/automations/sched-001/run/stream"); + expect(streamRes.status).toBe(200); + const body = String(streamRes.body); + expect(body).toContain("event: error"); + expect(body).not.toContain("event: complete"); + }); + it("defaults manual ai-prompt runs to all tools when allowedTools is omitted", async () => { vi.mocked(createFnAgent).mockClear(); const mockStore = createMockAutomationStore(); @@ -2057,6 +2094,56 @@ describe("Routine routes", () => { expect(runRes.body.result.output).toBe("routine-live-output"); }); + // FNXC:AutomationLiveOutput 2026-07-07-01:00 (FN-7652): mirrors the /automations/:id/run/stream + // coverage above — both stream endpoints share AutomationLiveRunRegistry/attachRun, so the + // runId-less-stream invariant (success run never delivers a terminal error) must hold for /routines too. + it("delivers complete, never a terminal error, for a runId-less stream attached after a successful run", async () => { + const mockStore = createMockRoutineStore(); + const { app } = buildRoutineApp(mockStore); + + const runRes = await REQUEST(app, "POST", "/api/routines/routine-001/run"); + expect(runRes.status).toBe(200); + expect(runRes.body.result.success).toBe(true); + + const streamRes = await performRequest(app, "GET", "/api/routines/routine-001/run/stream"); + expect(streamRes.status).toBe(200); + const body = String(streamRes.body); + expect(body).toContain("event: complete"); + expect(body).not.toContain("event: error"); + }); + + it("still delivers a real terminal error event for a runId-less stream after a genuinely failed run", async () => { + const mockStore = createMockRoutineStore(); + const routineRunner = createMockRoutineRunner(); + routineRunner.triggerManual.mockImplementation(async (_id: string, liveCallbacks?: { onStep?: (data: Record) => void }) => { + liveCallbacks?.onStep?.({ stepIndex: 0, stepId: "step-1", stepName: "Mock step", status: "started" }); + return { + routineId: "routine-001", + success: false, + output: "", + error: "synthetic routine failure", + triggerType: "cron" as const, + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + } satisfies RoutineExecutionResult; + }); + const store = createMockStore(); + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store, { routineStore: mockStore as any, routineRunner })); + + const runRes = await REQUEST(app, "POST", "/api/routines/routine-001/run"); + expect(runRes.status).toBe(200); + expect(runRes.body.result.success).toBe(false); + + const streamRes = await performRequest(app, "GET", "/api/routines/routine-001/run/stream"); + expect(streamRes.status).toBe(200); + const body = String(streamRes.body); + expect(body).toContain("event: error"); + expect(body).toContain("synthetic routine failure"); + expect(body).not.toContain("event: complete"); + }); + it("returns 404 for missing routine", async () => { const mockStore = createMockRoutineStore(); mockStore.getRoutine.mockRejectedValue(Object.assign(new Error("not found"), { code: "ENOENT" })); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index 33f593ebbd..67c57d90c2 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -2489,7 +2489,12 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout }); }; - const existingRun = automationLiveRuns.get(requestedRunId, schedule.id); + // FNXC:AutomationLiveOutput 2026-07-07-00:00 (FN-7652): no explicit runId means "attach me to + // this request's own run" — use getForAutoAttach so a stale finished run from before this + // trigger isn't mistaken for it (see AutomationLiveRunRegistry.getForAutoAttach). + const existingRun = requestedRunId + ? automationLiveRuns.get(requestedRunId, schedule.id) + : automationLiveRuns.getForAutoAttach(schedule.id); if (existingRun) { attachRun(existingRun); } else if (requestedRunId) { @@ -2977,7 +2982,12 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout }); }; - const existingRun = automationLiveRuns.get(requestedRunId, routine.id); + // FNXC:AutomationLiveOutput 2026-07-07-00:00 (FN-7652): no explicit runId means "attach me to + // this request's own run" — use getForAutoAttach so a stale finished run from before this + // trigger isn't mistaken for it (see AutomationLiveRunRegistry.getForAutoAttach). + const existingRun = requestedRunId + ? automationLiveRuns.get(requestedRunId, routine.id) + : automationLiveRuns.getForAutoAttach(routine.id); if (existingRun) { attachRun(existingRun); } else if (requestedRunId) { @@ -4994,6 +5004,14 @@ type AutomationLiveRunRecord = { listeners: Set<(event: AutomationLiveEvent, eventId: number) => void>; output: string; cleanupTimer?: NodeJS.Timeout; + /* + * FNXC:AutomationLiveOutput 2026-07-07-00:00 (FN-7652): + * Wall-clock start time (ms since epoch) used solely to decide whether a runId-less GET + * .../run/stream request should auto-attach to this run (see getForAutoAttach). Distinct from the + * result's own ISO `startedAt`/`completedAt` timestamps, which describe the automation's execution + * window, not stream-attach freshness. + */ + startedAt: number; }; function createAutomationRunId(): string { @@ -5036,6 +5054,17 @@ class AutomationLiveRunRegistry { private readonly latestRunBySchedule = new Map(); private readonly scheduleStartListeners = new Map void>>(); + /* + * FNXC:AutomationLiveOutput 2026-07-07-00:00 (FN-7652): + * A runId-less GET .../run/stream auto-attach must not pick up a run that finished well before this + * specific trigger (e.g. the previous manual run for the same schedule/routine, still within the + * AUTOMATION_LIVE_RUN_TTL_MS replay window). Auto-attaching to that stale run replays its own + * (unrelated) terminal `complete`/`error` event onto a brand-new trigger's stream, which is exactly + * the false "Run failed"-for-a-success-run bug (FN-7652). Bound how old a *finished* run may be and + * still be auto-attached; a still-`running` run has no age limit since it IS the in-flight trigger. + */ + private static readonly AUTO_ATTACH_STALE_WINDOW_MS = 10_000; + start(scheduleId: string, runId = createAutomationRunId()): AutomationLiveRunRecord { const run: AutomationLiveRunRecord = { runId, @@ -5044,6 +5073,7 @@ class AutomationLiveRunRegistry { buffer: new SessionEventBuffer(AUTOMATION_LIVE_EVENT_CAPACITY), listeners: new Set(), output: "", + startedAt: Date.now(), }; this.runs.set(runId, run); this.latestRunBySchedule.set(scheduleId, runId); @@ -5064,6 +5094,24 @@ class AutomationLiveRunRegistry { return latestRunId ? this.runs.get(latestRunId) : undefined; } + /* + * FNXC:AutomationLiveOutput 2026-07-07-00:00 (FN-7652): + * Used by GET .../run/stream instead of `get()` when the caller supplied no explicit runId. Returns + * the latest run for the schedule/routine only when it is still live, or finished recently enough + * (AUTO_ATTACH_STALE_WINDOW_MS) to plausibly be the run this very request is racing against. + * Otherwise returns undefined so the caller falls back to `subscribeToScheduleStart` and waits for + * its own fresh `run` event, instead of replaying an unrelated older run's terminal outcome. + */ + getForAutoAttach(scheduleId: string): AutomationLiveRunRecord | undefined { + const latestRunId = this.latestRunBySchedule.get(scheduleId); + if (!latestRunId) return undefined; + const run = this.runs.get(latestRunId); + if (!run) return undefined; + if (run.status === "running") return run; + if (Date.now() - run.startedAt < AutomationLiveRunRegistry.AUTO_ATTACH_STALE_WINDOW_MS) return run; + return undefined; + } + getBufferedEvents(runId: string, lastEventId = 0) { return this.runs.get(runId)?.buffer.getEventsSince(lastEventId) ?? []; }