feat(flows): add "Save & enable" to the Flow Canvas copilot proposal card (#5089)

This commit is contained in:
Cyrus Gray
2026-07-21 19:51:24 +03:00
committed by GitHub
parent 846db8aadc
commit f92888a5f5
18 changed files with 556 additions and 57 deletions
@@ -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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={onAccept}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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<void>(resolve => {
resolveSave = resolve;
});
const onAccept = vi.fn().mockReturnValue(savePromise);
hookState.proposal = proposalWith(['a', 'c']);
render(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={onAccept}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={onAccept}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={onAccept}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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
@@ -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<void>;
onAccept: (proposal: WorkflowProposal, opts?: { enable?: boolean }) => void | Promise<void>;
/** Reject the pending proposal (host reverts the overlay). */
onReject: () => void;
/** Close the panel. */
@@ -168,11 +174,25 @@ export default function WorkflowCopilotPanel({
const fileInputRef = useRef<HTMLInputElement | null>(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<WorkflowProposal | null>(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({
)}
</div>
<div className="mt-3 flex items-center gap-2">
<div className="mt-3 flex flex-wrap items-center gap-2">
<Button
type="button"
variant="primary"
size="sm"
disabled={acceptSaving}
analyticsId="workflow-copilot-accept"
disabled={acceptBusy}
data-testid="workflow-copilot-accept"
onClick={() => void accept()}>
{acceptSaving ? t('flows.copilot.saving') : t('flows.copilot.acceptAndSave')}
{acceptState === 'saving'
? t('flows.copilot.saving')
: t('flows.copilot.acceptAndSave')}
</Button>
<Button
type="button"
variant="primary"
size="sm"
analyticsId="workflow-copilot-accept-and-enable"
disabled={acceptBusy}
data-testid="workflow-copilot-accept-and-enable"
onClick={() => void acceptAndEnable()}>
{acceptState === 'enabling'
? t('flows.copilot.enabling')
: t('flows.copilot.saveAndEnable')}
</Button>
<Button
type="button"
variant="secondary"
size="sm"
disabled={acceptSaving}
disabled={acceptBusy}
data-testid="workflow-copilot-reject"
onClick={reject}>
{t('flows.copilot.reject')}
</Button>
</div>
{acceptState === 'idle' && enableError && (
<p className="mt-2 text-xs text-coral" data-testid="workflow-copilot-enable-error">
{t('flows.copilot.enableError')}
</p>
)}
</div>
)}
+3
View File
@@ -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': 'فشل تشغيل؛ راجعه واقترح إصلاحًا.',
+4
View File
@@ -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':
'একটি প্রস্তাবিত খসড়া পর্যালোচনা হচ্ছে: এখনও কিছু সংরক্ষণ করা হয়নি।',
+4
View File
@@ -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.',
+4
View File
@@ -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.',
+4
View File
@@ -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.',
+4
View File
@@ -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 dactiver le workflow. Essayez de lactiver depuis la liste.',
'flows.copilot.reject': 'Ignorer',
'flows.copilot.previewHint': 'Examen dun brouillon proposé: rien nest encore enregistré.',
'flows.copilot.repairDisplay': 'Une exécution a échoué ; examinez-la et proposez une correction.',
+4
View File
@@ -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':
'एक प्रस्तावित ड्राफ़्ट की समीक्षा हो रही है: अभी कुछ सहेजा नहीं गया।',
+4
View File
@@ -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.',
+4
View File
@@ -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': 'Unesecuzione è fallita; esaminala e proponi una correzione.',
+4
View File
@@ -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': '실행이 실패했습니다. 확인하고 수정을 제안하세요.',
+4
View File
@@ -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.',
+4
View File
@@ -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.',
+4
View File
@@ -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': 'Запуск завершился ошибкой; изучите его и предложите исправление.',
+3
View File
@@ -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': '一次运行失败了,请查看并提出修复方案。',
+118 -19
View File
@@ -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;
}
},
+188 -5
View File
@@ -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<void>)(
proposal
);
await (
copilotPanelProps.current?.onAccept as (
p: WorkflowProposal,
opts?: { enable?: boolean }
) => Promise<void>
)(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(
<MemoryRouter
initialEntries={[
{
pathname: '/flows/draft',
state: {
name: 'New workflow',
graph: {
schema_version: 1,
name: 'New workflow',
nodes: [
{
id: 't',
kind: 'trigger',
name: 'Start',
config: {},
ports: [],
position: { x: 0, y: 0 },
},
],
edges: [],
},
requireApproval: false,
},
},
]}>
<Routes>
<Route path="/flows/draft" element={<FlowCanvasDraftPage />} />
<Route path="/flows/:id" element={<FlowCanvasPage />} />
</Routes>
</MemoryRouter>
);
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(
<MemoryRouter
initialEntries={[
{
pathname: '/flows/draft',
state: {
name: 'New workflow',
graph: {
schema_version: 1,
name: 'New workflow',
nodes: [
{
id: 't',
kind: 'trigger',
name: 'Start',
config: {},
ports: [],
position: { x: 0, y: 0 },
},
],
edges: [],
},
requireApproval: false,
},
},
]}>
<Routes>
<Route path="/flows/draft" element={<FlowCanvasDraftPage />} />
<Route path="/flows/:id" element={<FlowCanvasPage />} />
</Routes>
</MemoryRouter>
);
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<void>
)(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<void>
)(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', () => {