From 85d89b24b83e7e4ef21900aa6f3ba94d729e8053 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 15:32:34 +0000 Subject: [PATCH] fix(automation): make creating from a recipe retry-safe too Review caught the recipe dialog having the same shape of bug as the form, one step earlier. addAutomation ran unconditionally on every attempt, so a trigger request that failed left an automation behind and pressing create again made a second one. With a recipe that binds more than one lifecycle, the cycles that had succeeded would then exist on both, and the show would fire them twice. The comment there claimed the half created automation was recoverable, which it was, but only by abandoning the dialog: the retry the error message offered was the thing that duplicated it. The dialog now records what it has already put on the server. A second attempt edits that automation rather than creating another, so parameters corrected after the failure are still applied, and adds only the triggers still missing. Verified by failing the first trigger request: the failed attempt leaves one automation with no trigger, and the retry reuses it and adds the trigger, rather than leaving two automations behind. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AfDKsy6PE3Rbyt32Fg4YKf --- .../automations-panel/NewAutomationDialog.tsx | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx index 24539e153..a4b7deea2 100644 --- a/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx +++ b/apps/client/src/features/app-settings/panel/automations-panel/NewAutomationDialog.tsx @@ -1,8 +1,8 @@ -import type { Automation } from 'ontime-types'; -import { useMemo, useState, type KeyboardEvent } from 'react'; +import type { Automation, TimerLifeCycle } from 'ontime-types'; +import { useMemo, useRef, useState, type KeyboardEvent } from 'react'; import { IoAdd, IoArrowBack, IoChevronForward, IoClose, IoSearch } from 'react-icons/io5'; -import { addAutomation, addTrigger } from '../../../../common/api/automation'; +import { addAutomation, addTrigger, editAutomation } from '../../../../common/api/automation'; import { maybeAxiosError } from '../../../../common/api/utils'; import Button from '../../../../common/components/buttons/Button'; import IconButton from '../../../../common/components/buttons/IconButton'; @@ -202,24 +202,42 @@ function RecipeSetup({ recipe, onClose, onBack, onCreated }: RecipeSetupProps) { const setValue = (name: string, value: string) => setValues((prev) => ({ ...prev, [name]: value })); + /** + * What this dialog has already put on the server. + * + * Creating takes one request per trigger on top of the automation itself, so a failure + * part way through leaves work already done. Recording it means pressing create again + * edits that automation and adds only the triggers still missing, rather than making a + * second automation and firing the same cycles twice. + */ + const created = useRef(null); + const createdCycles = useRef>(new Set()); + const handleCreate = async () => { setError(null); setIsCreating(true); try { - // the same two steps the automation form takes when it saves a new automation: - // the server generates the id, so the automation has to exist before a trigger can point at it - const created = await addAutomation(automation); + // the server generates the id, so the automation has to exist before a trigger points at it + const existing = created.current; + created.current = existing + ? await editAutomation(existing.id, { id: existing.id, ...automation }) + : await addAutomation(automation); + for (const cycle of recipe.triggers) { + if (createdCycles.current.has(cycle)) { + continue; + } await addTrigger({ title: makeTriggerTitle(automation.title, cycle), trigger: cycle, - automationId: created.id, + automationId: created.current.id, }); + createdCycles.current.add(cycle); } - onCreated(created); + onCreated(created.current); } catch (error) { - // a half created automation is visible in the list and flagged there, so say what - // happened and let the user finish it in the form rather than undoing their work + // what did land is a normal automation, visible in the list. Say what happened and let + // the user press create again rather than undoing work behind their back setError(maybeAxiosError(error)); } finally { setIsCreating(false);