From f92888a5f5151c6d95f511edaf928b318b7a4ac8 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:21:24 +0530 Subject: [PATCH] feat(flows): add "Save & enable" to the Flow Canvas copilot proposal card (#5089) --- .../flows/WorkflowCopilotPanel.test.tsx | 101 +++++++++ .../components/flows/WorkflowCopilotPanel.tsx | 128 +++++++++--- app/src/lib/i18n/ar.ts | 3 + app/src/lib/i18n/bn.ts | 4 + app/src/lib/i18n/de.ts | 4 + app/src/lib/i18n/en.ts | 4 + app/src/lib/i18n/es.ts | 4 + app/src/lib/i18n/fr.ts | 4 + app/src/lib/i18n/hi.ts | 4 + app/src/lib/i18n/id.ts | 4 + app/src/lib/i18n/it.ts | 4 + app/src/lib/i18n/ko.ts | 4 + app/src/lib/i18n/pl.ts | 4 + app/src/lib/i18n/pt.ts | 4 + app/src/lib/i18n/ru.ts | 4 + app/src/lib/i18n/zh-CN.ts | 3 + app/src/pages/FlowCanvasPage.tsx | 137 +++++++++++-- .../pages/__tests__/FlowCanvasPage.test.tsx | 193 +++++++++++++++++- 18 files changed, 556 insertions(+), 57 deletions(-) diff --git a/app/src/components/flows/WorkflowCopilotPanel.test.tsx b/app/src/components/flows/WorkflowCopilotPanel.test.tsx index 1e896b19b..5c589fb29 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.test.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.test.tsx @@ -244,6 +244,107 @@ describe('WorkflowCopilotPanel', () => { expect(hookState.clearProposal).not.toHaveBeenCalled(); }); + // PR1 — "Save & enable": a second button next to "Accept & save" that asks + // the host to save AND arm the flow in one click, mirroring the main-chat + // `WorkflowProposalCard`'s create+arm parity. + describe('Save & enable (PR1)', () => { + it('calls onAccept with { enable: true } and clears the proposal once it resolves', async () => { + const onAccept = vi.fn().mockResolvedValue(undefined); + hookState.proposal = proposalWith(['a', 'c']); + render( + + ); + fireEvent.click(screen.getByTestId('workflow-copilot-accept-and-enable')); + expect(onAccept).toHaveBeenCalledWith(hookState.proposal, { enable: true }); + await waitFor(() => expect(hookState.clearProposal).toHaveBeenCalledTimes(1)); + }); + + it('shows the enabling label and disables both accept buttons while the host save is in flight', async () => { + let resolveSave!: () => void; + const savePromise = new Promise(resolve => { + resolveSave = resolve; + }); + const onAccept = vi.fn().mockReturnValue(savePromise); + hookState.proposal = proposalWith(['a', 'c']); + render( + + ); + + fireEvent.click(screen.getByTestId('workflow-copilot-accept-and-enable')); + await waitFor(() => + expect(screen.getByTestId('workflow-copilot-accept-and-enable')).toHaveTextContent( + 'flows.copilot.enabling' + ) + ); + expect(screen.getByTestId('workflow-copilot-accept-and-enable')).toBeDisabled(); + // The plain "Accept & save" button must also be disabled while the + // enable-flavored save is in flight — the two must not race. + expect(screen.getByTestId('workflow-copilot-accept')).toBeDisabled(); + expect(hookState.clearProposal).not.toHaveBeenCalled(); + + resolveSave(); + await waitFor(() => expect(hookState.clearProposal).toHaveBeenCalledTimes(1)); + }); + + it('leaves the proposal visible and shows an enable-error message when the host save/enable rejects', async () => { + const onAccept = vi.fn().mockRejectedValue(new Error('enable failed')); + hookState.proposal = proposalWith(['a', 'c']); + render( + + ); + + fireEvent.click(screen.getByTestId('workflow-copilot-accept-and-enable')); + await waitFor(() => expect(onAccept).toHaveBeenCalledTimes(1)); + // The button re-enables once the rejected save settles, the proposal + // was never cleared (stays up for retry), and the dedicated enable-error + // message appears. + await waitFor(() => + expect(screen.getByTestId('workflow-copilot-accept-and-enable')).not.toBeDisabled() + ); + expect(hookState.clearProposal).not.toHaveBeenCalled(); + expect(screen.getByTestId('workflow-copilot-enable-error')).toHaveTextContent( + 'flows.copilot.enableError' + ); + }); + + it('does not show the enable-error message for a plain Accept & save failure', async () => { + const onAccept = vi.fn().mockRejectedValue(new Error('save failed')); + hookState.proposal = proposalWith(['a', 'c']); + render( + + ); + + fireEvent.click(screen.getByTestId('workflow-copilot-accept')); + await waitFor(() => expect(onAccept).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.getByTestId('workflow-copilot-accept')).not.toBeDisabled()); + expect(screen.queryByTestId('workflow-copilot-enable-error')).not.toBeInTheDocument(); + }); + }); + it('disables Reject while an Accept save is in flight, so it cannot race the persisted save', async () => { // Regression for the CodeRabbit finding: Reject must not stay clickable // while `onAccept`'s save is still pending, otherwise the user's cancel diff --git a/app/src/components/flows/WorkflowCopilotPanel.tsx b/app/src/components/flows/WorkflowCopilotPanel.tsx index af4a67ea3..e68beeb82 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.tsx @@ -75,8 +75,14 @@ interface Props { * persists it — "accept" is now review + save in one step). May return a * promise the panel awaits to show a saving state; a rejected promise * leaves the proposal visible so the user can retry. + * + * `opts.enable` (PR1 — "Save & enable") requests an immediate follow-up + * arm after the save succeeds, mirroring the main-chat + * `WorkflowProposalCard`'s one-click create+arm. Optional and backward + * compatible — a plain "Accept & save" click omits `opts` entirely, so it + * neither enables nor force-disables an already-enabled existing flow. */ - onAccept: (proposal: WorkflowProposal) => void | Promise; + onAccept: (proposal: WorkflowProposal, opts?: { enable?: boolean }) => void | Promise; /** Reject the pending proposal (host reverts the overlay). */ onReject: () => void; /** Close the panel. */ @@ -168,11 +174,25 @@ export default function WorkflowCopilotPanel({ const fileInputRef = useRef(null); const isComposingTextRef = useRef(false); + // Set only when a "Save & enable" attempt's `onAccept` rejects — surfaced + // as a dedicated inline message (`flows.copilot.enableError`) distinct from + // a plain "Accept & save" failure, which stays silent-but-retryable as + // before (the button re-enabling is signal enough there). Declared early + // (ahead of the proposal-surfacing effect below, which also clears it) so + // both that effect and the accept/reject handlers further down can + // reference it without a temporal-dead-zone ordering issue. + const [enableError, setEnableError] = useState(false); + // Surface each NEW proposal to the host exactly once (enter preview overlay). const lastSurfacedRef = useRef(null); useEffect(() => { if (proposal && proposal !== lastSurfacedRef.current) { lastSurfacedRef.current = proposal; + // A genuinely new proposal object replacing a prior one (e.g. a further + // revise turn) supersedes any stale "Save & enable" failure from the + // earlier proposal — clear it so the new card doesn't inherit an + // unrelated error message. + setEnableError(false); onProposal(proposal); } }, [proposal, onProposal]); @@ -376,39 +396,61 @@ export default function WorkflowCopilotPanel({ // Accept now review-and-saves: `onAccept` (the host's `handleAcceptProposal`) // applies the proposal to the draft AND persists it. Track a local - // `acceptSaving` flag so the button can show a saving state and disable - // re-clicks while that's in flight. If the host's save throws, leave the - // proposal card visible (don't `clearProposal()`) so the user can retry — - // otherwise a failed autosave would silently vanish the only affordance to - // try again from the copilot itself (the header Save button is a fallback, - // but this keeps the copilot's own flow self-contained). - const [acceptSaving, setAcceptSaving] = useState(false); - const accept = useCallback(async () => { - // Self-guard against re-entrance: the JSX `disabled={acceptSaving}` on - // the Accept button prevents a normal double-click, but `acceptSaving` - // only flips after the FIRST call's `setAcceptSaving(true)` commits — a - // second invocation racing ahead of that render (e.g. programmatic - // re-fire) must not start a second concurrent save. - if (!proposal || acceptSaving) return; - setAcceptSaving(true); - log('accept: saving proposal via host onAccept'); - try { - await onAccept(proposal); - log('accept: save succeeded, clearing proposal'); - clearProposal(); - lastSurfacedRef.current = null; - } catch (err) { - log('accept: save failed, leaving proposal visible for retry err=%o', err); - } finally { - setAcceptSaving(false); - } - }, [proposal, acceptSaving, onAccept, clearProposal]); + // `acceptState` union (rather than a plain boolean) so the two accept + // buttons ("Accept & save" / "Save & enable", PR1) can each show their own + // in-flight label while BOTH stay disabled — a save-in-flight click on the + // other button, or Reject, must not race the pending persist. If the + // host's save (or enable) throws, leave the proposal card visible (don't + // `clearProposal()`) so the user can retry — otherwise a failed autosave + // would silently vanish the only affordance to try again from the copilot + // itself (the header Save button is a fallback, but this keeps the + // copilot's own flow self-contained). + const [acceptState, setAcceptState] = useState<'idle' | 'saving' | 'enabling'>('idle'); + const acceptBusy = acceptState !== 'idle'; + const runAccept = useCallback( + async (opts?: { enable?: boolean }) => { + // Self-guard against re-entrance: the JSX `disabled={acceptBusy}` on + // both buttons prevents a normal double-click, but `acceptState` only + // flips after the FIRST call's `setAcceptState(...)` commits — a + // second invocation racing ahead of that render (e.g. programmatic + // re-fire) must not start a second concurrent save. + if (!proposal || acceptBusy) return; + const enable = Boolean(opts?.enable); + setAcceptState(enable ? 'enabling' : 'saving'); + setEnableError(false); + log('accept: saving proposal via host onAccept enable=%s', enable); + try { + // Plain "Accept & save" calls `onAccept` with just the proposal (no + // second argument at all) — matching the pre-PR1 call signature + // exactly — so a host that doesn't care about `opts` (or a caller + // asserting on `onAccept`'s exact arguments) sees no behavioral + // change. Only "Save & enable" adds the `{ enable: true }` opts. + if (enable) { + await onAccept(proposal, opts); + } else { + await onAccept(proposal); + } + log('accept: save succeeded, clearing proposal'); + clearProposal(); + lastSurfacedRef.current = null; + } catch (err) { + log('accept: save (or enable) failed, leaving proposal visible for retry err=%o', err); + if (enable) setEnableError(true); + } finally { + setAcceptState('idle'); + } + }, + [proposal, acceptBusy, onAccept, clearProposal, setEnableError] + ); + const accept = useCallback(() => runAccept(), [runAccept]); + const acceptAndEnable = useCallback(() => runAccept({ enable: true }), [runAccept]); const reject = useCallback(() => { onReject(); clearProposal(); lastSurfacedRef.current = null; - }, [onReject, clearProposal]); + setEnableError(false); + }, [onReject, clearProposal, setEnableError]); const diff = proposal ? diffGraphs(graph, proposal.graph as WorkflowGraph) : null; @@ -492,26 +534,46 @@ export default function WorkflowCopilotPanel({ )} -
+
+
+ {acceptState === 'idle' && enableError && ( +

+ {t('flows.copilot.enableError')} +

+ )}
)} diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 58cefa507..145b79cbf 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -4087,7 +4087,10 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'هذا الاقتراح لا يغيّر أي عقدة.', 'flows.copilot.accept': 'تطبيق على المسودة', 'flows.copilot.acceptAndSave': 'قبول وحفظ', + 'flows.copilot.saveAndEnable': 'حفظ وتفعيل', 'flows.copilot.saving': 'جارٍ الحفظ…', + 'flows.copilot.enabling': 'جارٍ التفعيل…', + 'flows.copilot.enableError': 'تم الحفظ، لكن تعذّر تفعيل سير العمل. حاول تفعيله من القائمة.', 'flows.copilot.reject': 'تجاهل', 'flows.copilot.previewHint': 'جارٍ مراجعة مسودة مقترحة: لم يُحفظ شيء بعد.', 'flows.copilot.repairDisplay': 'فشل تشغيل؛ راجعه واقترح إصلاحًا.', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 330052021..62f87ff14 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -4187,7 +4187,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'এই প্রস্তাব কোনো নোড পরিবর্তন করে না।', 'flows.copilot.accept': 'খসড়ায় প্রয়োগ করুন', 'flows.copilot.acceptAndSave': 'গ্রহণ ও সংরক্ষণ করুন', + 'flows.copilot.saveAndEnable': 'সংরক্ষণ ও সক্রিয় করুন', 'flows.copilot.saving': 'সংরক্ষণ করা হচ্ছে…', + 'flows.copilot.enabling': 'সক্রিয় করা হচ্ছে…', + 'flows.copilot.enableError': + 'সংরক্ষিত হয়েছে, কিন্তু ওয়ার্কফ্লো সক্রিয় করা যায়নি। তালিকা থেকে এটি চালু করার চেষ্টা করুন।', 'flows.copilot.reject': 'বাতিল করুন', 'flows.copilot.previewHint': 'একটি প্রস্তাবিত খসড়া পর্যালোচনা হচ্ছে: এখনও কিছু সংরক্ষণ করা হয়নি।', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 5b83faec0..630b63ddf 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -4306,7 +4306,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Dieser Vorschlag ändert keine Knoten.', 'flows.copilot.accept': 'Auf Entwurf anwenden', 'flows.copilot.acceptAndSave': 'Übernehmen & speichern', + 'flows.copilot.saveAndEnable': 'Speichern & aktivieren', 'flows.copilot.saving': 'Wird gespeichert…', + 'flows.copilot.enabling': 'Wird aktiviert…', + 'flows.copilot.enableError': + 'Workflow gespeichert, konnte aber nicht aktiviert werden. Versuchen Sie es erneut oder aktivieren Sie ihn auf der Workflows-Seite.', 'flows.copilot.reject': 'Verwerfen', 'flows.copilot.previewHint': 'Ein vorgeschlagener Entwurf wird geprüft: es wurde noch nichts gespeichert.', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 3368ea52c..036908c71 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -4880,7 +4880,11 @@ const en: TranslationMap = { 'flows.copilot.noChanges': 'No node changes in this proposal.', 'flows.copilot.accept': 'Apply to draft', 'flows.copilot.acceptAndSave': 'Accept & save', + 'flows.copilot.saveAndEnable': 'Save & enable', 'flows.copilot.saving': 'Saving…', + 'flows.copilot.enabling': 'Enabling…', + 'flows.copilot.enableError': + 'Saved, but could not enable the workflow. Try toggling it on from the list.', 'flows.copilot.reject': 'Dismiss', 'flows.copilot.previewHint': 'Reviewing a proposed draft: nothing is saved yet.', 'flows.copilot.repairDisplay': 'A run failed. Please review it and propose a fix.', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index 870fc00a0..7254c56c3 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -4259,7 +4259,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Esta propuesta no cambia ningún nodo.', 'flows.copilot.accept': 'Aplicar al borrador', 'flows.copilot.acceptAndSave': 'Aceptar y guardar', + 'flows.copilot.saveAndEnable': 'Guardar y activar', 'flows.copilot.saving': 'Guardando…', + 'flows.copilot.enabling': 'Activando…', + 'flows.copilot.enableError': + 'Guardado, pero no se pudo activar el flujo de trabajo. Actívalo desde la lista.', 'flows.copilot.reject': 'Descartar', 'flows.copilot.previewHint': 'Revisando un borrador propuesto: aún no se ha guardado nada.', 'flows.copilot.repairDisplay': 'Falló una ejecución; revísala y propón una solución.', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index 2a4b49ee0..41e7d09a0 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -4287,7 +4287,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Cette proposition ne modifie aucun nœud.', 'flows.copilot.accept': 'Appliquer au brouillon', 'flows.copilot.acceptAndSave': 'Accepter et enregistrer', + 'flows.copilot.saveAndEnable': 'Enregistrer et activer', 'flows.copilot.saving': 'Enregistrement…', + 'flows.copilot.enabling': 'Activation…', + 'flows.copilot.enableError': + 'Enregistré, mais impossible d’activer le workflow. Essayez de l’activer depuis la liste.', 'flows.copilot.reject': 'Ignorer', 'flows.copilot.previewHint': 'Examen d’un brouillon proposé: rien n’est encore enregistré.', 'flows.copilot.repairDisplay': 'Une exécution a échoué ; examinez-la et proposez une correction.', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index be21c6c87..79138ea39 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -4185,7 +4185,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'यह प्रस्ताव किसी नोड को नहीं बदलता।', 'flows.copilot.accept': 'ड्राफ़्ट पर लागू करें', 'flows.copilot.acceptAndSave': 'स्वीकार करें और सहेजें', + 'flows.copilot.saveAndEnable': 'सहेजें और सक्रिय करें', 'flows.copilot.saving': 'सहेजा जा रहा है…', + 'flows.copilot.enabling': 'सक्रिय किया जा रहा है…', + 'flows.copilot.enableError': + 'सहेज लिया गया है, लेकिन वर्कफ़्लो सक्रिय नहीं हो सका। सूची से इसे चालू करने का प्रयास करें।', 'flows.copilot.reject': 'रद्द करें', 'flows.copilot.previewHint': 'एक प्रस्तावित ड्राफ़्ट की समीक्षा हो रही है: अभी कुछ सहेजा नहीं गया।', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 1a4a83723..46b4fb742 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -4202,7 +4202,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Usulan ini tidak mengubah simpul apa pun.', 'flows.copilot.accept': 'Terapkan ke draf', 'flows.copilot.acceptAndSave': 'Terima & simpan', + 'flows.copilot.saveAndEnable': 'Simpan & aktifkan', 'flows.copilot.saving': 'Menyimpan…', + 'flows.copilot.enabling': 'Mengaktifkan…', + 'flows.copilot.enableError': + 'Tersimpan, tetapi alur kerja tidak dapat diaktifkan. Coba aktifkan dari daftar.', 'flows.copilot.reject': 'Buang', 'flows.copilot.previewHint': 'Meninjau draf yang diusulkan: belum ada yang disimpan.', 'flows.copilot.repairDisplay': 'Sebuah eksekusi gagal; periksa dan usulkan perbaikan.', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index d0e6c1bdf..9a86d5e14 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -4256,7 +4256,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Questa proposta non modifica alcun nodo.', 'flows.copilot.accept': 'Applica alla bozza', 'flows.copilot.acceptAndSave': 'Accetta e salva', + 'flows.copilot.saveAndEnable': 'Salva e attiva', 'flows.copilot.saving': 'Salvataggio…', + 'flows.copilot.enabling': 'Attivazione…', + 'flows.copilot.enableError': + 'Workflow salvato, ma non è stato possibile attivarlo. Riprova, oppure attivalo dalla pagina Workflows.', 'flows.copilot.reject': 'Ignora', 'flows.copilot.previewHint': 'Revisione di una bozza proposta: non è stato ancora salvato nulla.', 'flows.copilot.repairDisplay': 'Un’esecuzione è fallita; esaminala e proponi una correzione.', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index 315bf4b4d..1620ecf3d 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -4140,7 +4140,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': '이 제안은 노드를 변경하지 않습니다.', 'flows.copilot.accept': '초안에 적용', 'flows.copilot.acceptAndSave': '수락 및 저장', + 'flows.copilot.saveAndEnable': '저장 및 활성화', 'flows.copilot.saving': '저장 중…', + 'flows.copilot.enabling': '활성화 중…', + 'flows.copilot.enableError': + '저장되었지만 워크플로를 활성화하지 못했습니다. 목록에서 활성화해 보세요.', 'flows.copilot.reject': '버리기', 'flows.copilot.previewHint': '제안된 초안을 검토 중입니다: 아직 저장되지 않았습니다.', 'flows.copilot.repairDisplay': '실행이 실패했습니다. 확인하고 수정을 제안하세요.', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index 51838b6e9..4a5dda9f2 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -4241,7 +4241,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Ta propozycja nie zmienia żadnego węzła.', 'flows.copilot.accept': 'Zastosuj do wersji roboczej', 'flows.copilot.acceptAndSave': 'Zaakceptuj i zapisz', + 'flows.copilot.saveAndEnable': 'Zapisz i włącz', 'flows.copilot.saving': 'Zapisywanie…', + 'flows.copilot.enabling': 'Włączanie…', + 'flows.copilot.enableError': + 'Zapisano, ale nie udało się włączyć przepływu pracy. Spróbuj włączyć go z listy.', 'flows.copilot.reject': 'Odrzuć', 'flows.copilot.previewHint': 'Przeglądasz proponowaną wersję roboczą: nic nie zostało jeszcze zapisane.', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 4faab0c69..712be4fbb 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -4248,7 +4248,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Esta proposta não altera nenhum nó.', 'flows.copilot.accept': 'Aplicar ao rascunho', 'flows.copilot.acceptAndSave': 'Aceitar e salvar', + 'flows.copilot.saveAndEnable': 'Salvar e ativar', 'flows.copilot.saving': 'Salvando…', + 'flows.copilot.enabling': 'Ativando…', + 'flows.copilot.enableError': + 'Salvo, mas não foi possível ativar o fluxo de trabalho. Tente ativá-lo pela lista.', 'flows.copilot.reject': 'Descartar', 'flows.copilot.previewHint': 'Revisando um rascunho proposto: nada foi salvo ainda.', 'flows.copilot.repairDisplay': 'Uma execução falhou; analise-a e proponha uma correção.', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 5aa939b2c..5bf1c48d9 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -4226,7 +4226,11 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': 'Это предложение не меняет ни одного узла.', 'flows.copilot.accept': 'Применить к черновику', 'flows.copilot.acceptAndSave': 'Принять и сохранить', + 'flows.copilot.saveAndEnable': 'Сохранить и включить', 'flows.copilot.saving': 'Сохранение…', + 'flows.copilot.enabling': 'Включение…', + 'flows.copilot.enableError': + 'Сохранено, но не удалось включить рабочий процесс. Попробуйте включить его из списка.', 'flows.copilot.reject': 'Отклонить', 'flows.copilot.previewHint': 'Просмотр предложенного черновика: пока ничего не сохранено.', 'flows.copilot.repairDisplay': 'Запуск завершился ошибкой; изучите его и предложите исправление.', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index e86dfac2a..4861f5a83 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -3967,7 +3967,10 @@ const messages: TranslationMap = { 'flows.copilot.noChanges': '此方案未更改任何节点。', 'flows.copilot.accept': '应用到草稿', 'flows.copilot.acceptAndSave': '接受并保存', + 'flows.copilot.saveAndEnable': '保存并启用', 'flows.copilot.saving': '保存中…', + 'flows.copilot.enabling': '启用中…', + 'flows.copilot.enableError': '已保存，但无法启用该工作流。请尝试从列表中启用它。', 'flows.copilot.reject': '放弃', 'flows.copilot.previewHint': '正在查看建议的草稿：尚未保存任何内容。', 'flows.copilot.repairDisplay': '一次运行失败了，请查看并提出修复方案。', diff --git a/app/src/pages/FlowCanvasPage.tsx b/app/src/pages/FlowCanvasPage.tsx index 7f8c793c3..a7f42381f 100644 --- a/app/src/pages/FlowCanvasPage.tsx +++ b/app/src/pages/FlowCanvasPage.tsx @@ -44,7 +44,14 @@ import { workflowGraphToXyflow } from '../lib/flows/graphAdapter'; import { buildPreviewGraph, diffGraphs } from '../lib/flows/graphDiff'; import type { WorkflowGraph } from '../lib/flows/types'; import { useT } from '../lib/i18n/I18nContext'; -import { createFlow, type Flow, getFlow, runFlow, updateFlow } from '../services/api/flowsApi'; +import { + createFlow, + type Flow, + getFlow, + runFlow, + setFlowEnabled, + updateFlow, +} from '../services/api/flowsApi'; import type { WorkflowProposal } from '../store/chatRuntimeSlice'; import type { ToastNotification } from '../types/intelligence'; @@ -480,12 +487,30 @@ function FlowEditor({ // // Declared ahead of `handleAcceptProposal` (below), which calls it directly // to persist an accepted proposal immediately. + // + // Returns `flowId`/`flowEnabled` alongside `remounted` so a caller wanting a + // "Save & enable" follow-up (`handleAcceptProposal`'s `opts.enable`) knows + // exactly which flow id to arm and whether the persisted flow already came + // back enabled — without having to re-derive it from component state, which + // is especially important for the draft-create path: `flowId` (the prop) + // is still `null` in THIS closure even after `createFlow` resolves, since + // the draft only becomes a real flow id via the `navigate(...)` below, not + // a state update this same render can observe. const handleSave = useCallback( async ( next: WorkflowGraph, overrideName?: string, - overrideRequireApproval?: boolean - ): Promise<{ remounted: boolean }> => { + overrideRequireApproval?: boolean, + // When true, a draft-create does NOT navigate to `/flows/:id` itself — + // the caller owns navigation timing. `handleAcceptProposal`'s + // "Save & enable" needs this: it must run `setFlowEnabled` on the + // just-created flow BEFORE the route change unmounts this page, else the + // enable RPC resolves against an unmounted component (its loading/error + // state is lost and the new page shows the flow still disabled). The + // `wasDraft` flag in the return tells the caller navigation is now its + // responsibility. + deferDraftNavigation?: boolean + ): Promise<{ remounted: boolean; flowId: string; flowEnabled: boolean; wasDraft: boolean }> => { // `overrideName` covers the copilot-Accept call site: it calls // `setName(proposal.name)` and `handleSave(...)` in the same handler, // but `name` in THIS closure is still the pre-update value — React @@ -510,11 +535,24 @@ function FlowEditor({ effectiveRequireApproval ); const created = await createFlow(effectiveName, next, effectiveRequireApproval); - log('save: draft persisted as flow id=%s', created.id); - navigate(`/flows/${created.id}`, { replace: true }); + log('save: draft persisted as flow id=%s enabled=%s', created.id, created.enabled); + if (!deferDraftNavigation) { + navigate(`/flows/${created.id}`, { replace: true }); + } // Navigating replaces this whole page (new `flowId` route param), so // "remounted" is moot for a draft-create — no caller branches on it. - return { remounted: false }; + // `flowId`/`flowEnabled` DO matter — a "Save & enable" caller reads + // them to arm the just-created flow (B29 Rule 1 always persists an + // automatic-trigger draft disabled, regardless of the caller's + // intent), and this RPC response is the only place that id/enabled + // pair is available before the route change lands. `wasDraft` lets a + // `deferDraftNavigation` caller know it now owns the navigation. + return { + remounted: false, + flowId: created.id, + flowEnabled: created.enabled, + wasDraft: true, + }; } // Only include `name` / `requireApproval` in the update payload when // they actually diverge from what's already persisted (a manual @@ -565,13 +603,14 @@ function FlowEditor({ setCanvasVersion(v => v + 1); } log( - 'save: flow id=%s persisted — canvas re-synced from response nodes=%d edges=%d graphChanged=%s', + 'save: flow id=%s persisted — canvas re-synced from response nodes=%d edges=%d graphChanged=%s enabled=%s', flowId, persisted.nodes.length, persisted.edges.length, - graphChanged + graphChanged, + updated.enabled ); - return { remounted: graphChanged }; + return { remounted: graphChanged, flowId, flowEnabled: updated.enabled, wasDraft: false }; }, [isDraft, flowId, name, requireApproval, navigate] ); @@ -624,9 +663,20 @@ function FlowEditor({ // right after Accept would have done. A failed save is non-fatal: the // proposal stays applied to the (now dirty) draft and the header Save // button remains the manual retry — we never crash or revert the draft. + // + // `opts.enable` (PR1 — "Save & enable") mirrors `WorkflowProposalCard.save()` + // in the main chat surface: after a successful save, explicitly arm the + // flow via `setFlowEnabled`. This is needed because `createFlow` with an + // automatic trigger (schedule/app_event/webhook) ALWAYS persists disabled + // (B29 Rule 1, `flowsApi.ts`) regardless of what the caller passed — Rule 1 + // exists to stop a copilot autosave from silently arming an unattended + // automation, but "Save & enable" is the user's own explicit arming click, + // not a silent autosave, so it must follow up. Plain "Accept & save" (no + // `opts`) must NOT enable and must NOT force-disable an already-enabled + // existing flow — it's simply omitted from the call. const handleAcceptProposal = useCallback( - async (proposal: WorkflowProposal) => { - log('copilot proposal accepted'); + async (proposal: WorkflowProposal, opts?: { enable?: boolean }) => { + log('copilot proposal accepted: enable=%s', Boolean(opts?.enable)); const proposedGraph = proposal.graph as WorkflowGraph; setDraftGraph(proposedGraph); setPreview(null); @@ -670,10 +720,18 @@ function FlowEditor({ // `canvasVersion` bump above) so the ref's imperative handle is stale; // call `handleSave` directly with the known-good proposed graph. try { - const { remounted } = await handleSave( + const { + remounted, + flowId: savedFlowId, + flowEnabled, + wasDraft, + } = await handleSave( proposedGraph, overrideName, - proposal.requireApproval + proposal.requireApproval, + // Defer a draft-create's navigation so a "Save & enable" arms the + // flow BEFORE this page unmounts — see `deferDraftNavigation`. + true ); // The canvas remounted once already (this handler's own bump above) // with `forcedDirty` seeded `true` — correct pre-persist, but that @@ -688,15 +746,56 @@ function FlowEditor({ if (!remounted) { canvasRef.current?.clearForcedDirty(); } - log('copilot proposal accepted: persisted remounted=%s', remounted); + log( + 'copilot proposal accepted: persisted remounted=%s flowId=%s flowEnabled=%s', + remounted, + savedFlowId, + flowEnabled + ); + + // "Save & enable": follow up with an explicit arm, same as + // `WorkflowProposalCard.save()`. Fires unconditionally when + // requested (idempotent if the flow already came back enabled) — + // simpler than special-casing an already-enabled flow, and this is + // still inside the same try/catch so a failure here also leaves the + // proposal visible for retry rather than silently vanishing. + if (opts?.enable) { + log('copilot proposal accepted: enabling flow id=%s', savedFlowId); + try { + await setFlowEnabled(savedFlowId, true); + log('copilot proposal accepted: enable succeeded id=%s', savedFlowId); + } catch (enableErr) { + // The flow IS saved at this point. On a DRAFT we must still + // navigate to the created flow (below) or a retry would create a + // duplicate — so we can't keep the proposal for an in-place retry; + // swallow here and let the user arm it from the flow page (matches + // the "Saved, but could not enable" guidance). On an EXISTING flow + // there's no navigation, so rethrow to keep the proposal visible + // for retry, preserving the pre-existing behavior. + if (!wasDraft) throw enableErr; + log( + 'copilot proposal accepted: enable failed on draft; flow saved-but-disabled id=%s err=%o', + savedFlowId, + enableErr + ); + } + } + + // Draft navigation was deferred so the "Save & enable" arm could run + // first; now that persist + enable have settled, move to the real flow + // route. A non-draft accept stays on its existing `/flows/:id` page. + if (wasDraft) { + navigate(`/flows/${savedFlowId}`, { replace: true }); + } } catch (err) { - log('copilot proposal accepted: save failed err=%o', err); + log('copilot proposal accepted: save/enable failed err=%o', err); // Rethrow: the draft above is already applied unconditionally, so no // data is lost by rethrowing. This lets the caller — the copilot - // panel's own `accept` handler — see the failure and skip - // `clearProposal()`, keeping the proposal card visible for retry - // instead of silently vanishing while nothing was actually saved. - // `acceptSaving` there still resets via its own `finally`. + // panel's own `accept`/`acceptAndEnable` handler — see the failure + // and skip `clearProposal()`, keeping the proposal card visible for + // retry instead of silently vanishing while nothing was actually + // saved (or saved-but-not-enabled). `acceptSaving`/`acceptState` + // there still resets via its own `finally`. throw err; } }, diff --git a/app/src/pages/__tests__/FlowCanvasPage.test.tsx b/app/src/pages/__tests__/FlowCanvasPage.test.tsx index 6cbe4a2ca..28596e9eb 100644 --- a/app/src/pages/__tests__/FlowCanvasPage.test.tsx +++ b/app/src/pages/__tests__/FlowCanvasPage.test.tsx @@ -25,6 +25,7 @@ const createFlow = vi.hoisted(() => vi.fn()); const validateFlow = vi.hoisted(() => vi.fn()); const listFlowConnections = vi.hoisted(() => vi.fn()); const runFlow = vi.hoisted(() => vi.fn()); +const setFlowEnabled = vi.hoisted(() => vi.fn()); vi.mock('../../services/api/flowsApi', () => ({ getFlow, updateFlow, @@ -32,6 +33,7 @@ vi.mock('../../services/api/flowsApi', () => ({ validateFlow, listFlowConnections, runFlow, + setFlowEnabled, })); // Stub the copilot panel: it drives the real chat runtime (redux + socket), @@ -99,10 +101,12 @@ describe('FlowCanvasPage', () => { validateFlow.mockReset(); listFlowConnections.mockReset(); runFlow.mockReset(); + setFlowEnabled.mockReset(); validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] }); listFlowConnections.mockResolvedValue([]); updateFlow.mockResolvedValue(makeFlow()); createFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Daily digest' })); + setFlowEnabled.mockResolvedValue(makeFlow({ enabled: true })); }); it('shows a loading state while the flow is being fetched', () => { @@ -687,6 +691,7 @@ describe('FlowCanvasPage copilot proposal name adoption', () => { createFlow.mockReset(); validateFlow.mockReset(); listFlowConnections.mockReset(); + setFlowEnabled.mockReset(); validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] }); listFlowConnections.mockResolvedValue([]); // Accept now persists immediately (review + save in one step, see @@ -695,6 +700,7 @@ describe('FlowCanvasPage copilot proposal name adoption', () => { // by an unmocked (`undefined`-resolving) `updateFlow`/`createFlow`. updateFlow.mockResolvedValue(makeFlow()); createFlow.mockResolvedValue(makeFlow({ id: 'created-id' })); + setFlowEnabled.mockResolvedValue(makeFlow({ enabled: true })); }); function renderEditor(id = 'test-id') { @@ -710,12 +716,20 @@ describe('FlowCanvasPage copilot proposal name adoption', () => { // `handleAcceptProposal` is async (it awaits the persist call) — drive it // through `act(async () => …)` so React flushes every state update the - // resulting save produces before the test asserts on them. - function acceptProposal(proposal: WorkflowProposal = makeProposal()) { + // resulting save produces before the test asserts on them. `opts` mirrors + // the copilot panel's own "Save & enable" call (PR1) — omitted for a plain + // Accept & save, `{ enable: true }` for the enable path. + function acceptProposal( + proposal: WorkflowProposal = makeProposal(), + opts?: { enable?: boolean } + ) { return act(async () => { - await (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => Promise)( - proposal - ); + await ( + copilotPanelProps.current?.onAccept as ( + p: WorkflowProposal, + opts?: { enable?: boolean } + ) => Promise + )(proposal, opts); }); } @@ -984,6 +998,175 @@ describe('FlowCanvasPage copilot proposal name adoption', () => { await waitFor(() => expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument()); await waitFor(() => expect(screen.getByTestId('flow-editor-save')).not.toBeDisabled()); }); + + // PR1 — "Save & enable": `handleAcceptProposal`'s `opts.enable` follow-up. + describe('Save & enable (PR1)', () => { + it('calls setFlowEnabled(flowId, true) after a successful save on an existing flow', async () => { + getFlow.mockResolvedValue(makeFlow({ id: 'test-id', enabled: false })); + updateFlow.mockResolvedValue(makeFlow({ id: 'test-id', enabled: false })); + renderEditor(); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + await acceptProposal(makeProposal(), { enable: true }); + + expect(updateFlow).toHaveBeenCalledTimes(1); + expect(setFlowEnabled).toHaveBeenCalledTimes(1); + expect(setFlowEnabled).toHaveBeenCalledWith('test-id', true); + }); + + it('calls setFlowEnabled with the newly-created id on a draft', async () => { + createFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' })); + getFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' })); + render( + + + } /> + } /> + + + ); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + await acceptProposal(makeProposal(), { enable: true }); + + expect(createFlow).toHaveBeenCalledTimes(1); + await waitFor(() => expect(setFlowEnabled).toHaveBeenCalledTimes(1)); + expect(setFlowEnabled).toHaveBeenCalledWith('created-id', true); + }); + + it('on a draft "Save & enable", runs enable BEFORE navigating, and swallows an enable failure (flow saved, armed from its own page)', async () => { + createFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' })); + getFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' })); + setFlowEnabled.mockRejectedValue(new Error('enable rpc failed')); + render( + + + } /> + } /> + + + ); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + let caughtErr: unknown; + await act(async () => { + try { + await ( + copilotPanelProps.current?.onAccept as ( + p: WorkflowProposal, + opts?: { enable?: boolean } + ) => Promise + )(makeProposal(), { enable: true }); + } catch (err) { + caughtErr = err; + } + }); + + // On a draft the create succeeds first, so the enable is attempted + // BEFORE the deferred navigation (the whole point of the fix — otherwise + // navigate would unmount this page and the enable RPC would resolve + // against a dead component). And because the flow IS saved, a draft + // enable failure must NOT rethrow: rethrowing would strand the user on + // the draft and a retry would create a DUPLICATE flow. Instead we + // navigate to the real flow and let the user arm it there. (Contrast the + // existing-flow rethrow test above, which keeps the proposal for retry.) + expect(createFlow).toHaveBeenCalledTimes(1); + await waitFor(() => expect(setFlowEnabled).toHaveBeenCalledWith('created-id', true)); + expect(caughtErr).toBeUndefined(); + // Navigation to the real flow happened afterward (its page fetches it). + await waitFor(() => expect(getFlow).toHaveBeenCalledWith('created-id')); + }); + + it('does NOT call setFlowEnabled for a plain Accept & save (no opts)', async () => { + getFlow.mockResolvedValue(makeFlow({ id: 'test-id' })); + renderEditor(); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + await acceptProposal(); + + expect(updateFlow).toHaveBeenCalledTimes(1); + expect(setFlowEnabled).not.toHaveBeenCalled(); + }); + + it('rethrows an enable failure after a successful save, so the saved flow is not lost and the caller can retry', async () => { + getFlow.mockResolvedValue(makeFlow({ id: 'test-id' })); + updateFlow.mockResolvedValue(makeFlow({ id: 'test-id' })); + setFlowEnabled.mockRejectedValue(new Error('enable rpc failed')); + renderEditor(); + await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument()); + + let caughtErr: unknown; + await act(async () => { + try { + await ( + copilotPanelProps.current?.onAccept as ( + p: WorkflowProposal, + opts?: { enable?: boolean } + ) => Promise + )(makeProposal(), { enable: true }); + } catch (err) { + caughtErr = err; + } + }); + + // The save itself succeeded — `updateFlow` was called and resolved — + // only the follow-up enable call failed. Rethrowing lets the copilot + // panel's own catch branch skip `clearProposal()`, keeping the card + // visible for retry (matching the plain-save failure contract). + expect(updateFlow).toHaveBeenCalledTimes(1); + expect(setFlowEnabled).toHaveBeenCalledTimes(1); + expect(caughtErr).toBeInstanceOf(Error); + expect((caughtErr as Error).message).toBe('enable rpc failed'); + }); + }); }); describe('asCopilotBuildSeed', () => {