fix(flows): canvas Accept persists the proposal (accept = review + save) (#4893)

app/src/components/flows/WorkflowCopilotPanel.test.tsx

@@ -301,8 +301,8 @@ describe('WorkflowCopilotPanel', () => {
     expect(screen.getByTestId('workflow-copilot-removed')).toBeInTheDocument();
   });
 
-  it('Accept applies to the draft and clears the proposal (never persists)', () => {
-    const onAccept = vi.fn();
+  it('Accept calls onAccept (host applies + saves) and clears the proposal once it resolves', async () => {
+    const onAccept = vi.fn().mockResolvedValue(undefined);
     hookState.proposal = proposalWith(['a', 'c']);
     render(
       <WorkflowCopilotPanel
@@ -315,7 +315,94 @@ describe('WorkflowCopilotPanel', () => {
     );
     fireEvent.click(screen.getByTestId('workflow-copilot-accept'));
     expect(onAccept).toHaveBeenCalledWith(hookState.proposal);
-    expect(hookState.clearProposal).toHaveBeenCalledTimes(1);
+    await waitFor(() => expect(hookState.clearProposal).toHaveBeenCalledTimes(1));
+  });
+
+  it('shows the saving label and disables Accept while the host save is in flight', async () => {
+    // Deferred promise so the test controls exactly when the host's save
+    // (`onAccept`) resolves, to observe the in-between "saving" state.
+    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'));
+    await waitFor(() =>
+      expect(screen.getByTestId('workflow-copilot-accept')).toHaveTextContent(
+        'flows.copilot.saving'
+      )
+    );
+    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 for retry when the host save rejects', 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));
+    // The button re-enables once the rejected save settles, and the proposal
+    // was never cleared — the card stays up so the user can retry.
+    await waitFor(() => expect(screen.getByTestId('workflow-copilot-accept')).not.toBeDisabled());
+    expect(hookState.clearProposal).not.toHaveBeenCalled();
+  });
+
+  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
+    // can be silently overridden by the earlier Accept's save landing after.
+    let resolveSave!: () => void;
+    const savePromise = new Promise<void>(resolve => {
+      resolveSave = resolve;
+    });
+    const onAccept = vi.fn().mockReturnValue(savePromise);
+    const onReject = vi.fn();
+    hookState.proposal = proposalWith(['a', 'c']);
+    render(
+      <WorkflowCopilotPanel
+        graph={baseGraph}
+        onProposal={vi.fn()}
+        onAccept={onAccept}
+        onReject={onReject}
+        onClose={vi.fn()}
+      />
+    );
+
+    fireEvent.click(screen.getByTestId('workflow-copilot-accept'));
+    await waitFor(() => expect(screen.getByTestId('workflow-copilot-reject')).toBeDisabled());
+
+    // A click while disabled is a no-op in jsdom/RTL — Reject must not fire.
+    fireEvent.click(screen.getByTestId('workflow-copilot-reject'));
+    expect(onReject).not.toHaveBeenCalled();
+    expect(hookState.clearProposal).not.toHaveBeenCalled();
+
+    resolveSave();
+    await waitFor(() => expect(hookState.clearProposal).toHaveBeenCalledTimes(1));
+    expect(screen.getByTestId('workflow-copilot-reject')).not.toBeDisabled();
   });
 
   it('Reject discards the proposal without applying it', () => {

app/src/components/flows/WorkflowCopilotPanel.tsx

@@ -16,8 +16,13 @@
  * `streamingAssistantByThread`, streamed here by Phase B). So the copilot reads
  * like a real chat rather than a one-shot form.
  *
- * Invariant: the copilot only PROPOSES. Accept applies to the UNSAVED local
- * draft (no `flows_update`); persistence stays behind the canvas's own Save.
+ * Invariant: the copilot only PROPOSES — the agent turn itself never
+ * persists. Accept applies the proposal to the local draft AND immediately
+ * saves it (review + save in one click) via the host's `onAccept`, which
+ * awaits the host's own persistence call; the panel shows a saving state
+ * meanwhile and, if the save fails, leaves the proposal visible for retry
+ * rather than silently discarding it. Reject remains local-only (revert the
+ * overlay, no persistence call).
  */
 import createDebug from 'debug';
 import { useCallback, useEffect, useRef, useState } from 'react';
@@ -65,8 +70,13 @@ interface Props {
    * reflects it.
    */
   onProposal: (proposal: WorkflowProposal) => void;
-  /** Accept the pending proposal into the local draft (host commits it). */
-  onAccept: (proposal: WorkflowProposal) => void;
+  /**
+   * Accept the pending proposal (host applies it to the local draft AND
+   * 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.
+   */
+  onAccept: (proposal: WorkflowProposal) => void | Promise<void>;
   /** Reject the pending proposal (host reverts the overlay). */
   onReject: () => void;
   /** Close the panel. */
@@ -376,12 +386,35 @@ export default function WorkflowCopilotPanel({
   const noopAttach = useCallback(async () => {}, []);
   const noop = useCallback(() => {}, []);
 
-  const accept = useCallback(() => {
-    if (!proposal) return;
-    onAccept(proposal);
-    clearProposal();
-    lastSurfacedRef.current = null;
-  }, [proposal, onAccept, clearProposal]);
+  // 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]);
 
   const reject = useCallback(() => {
     onReject();
@@ -533,14 +566,16 @@ export default function WorkflowCopilotPanel({
                 type="button"
                 variant="primary"
                 size="sm"
+                disabled={acceptSaving}
                 data-testid="workflow-copilot-accept"
-                onClick={accept}>
-                {t('flows.copilot.accept')}
+                onClick={() => void accept()}>
+                {acceptSaving ? t('flows.copilot.saving') : t('flows.copilot.acceptAndSave')}
               </Button>
               <Button
                 type="button"
                 variant="secondary"
                 size="sm"
+                disabled={acceptSaving}
                 data-testid="workflow-copilot-reject"
                 onClick={reject}>
                 {t('flows.copilot.reject')}

app/src/lib/i18n/ar.ts

@@ -4014,6 +4014,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': 'أُزيل {count}',
   'flows.copilot.noChanges': 'هذا الاقتراح لا يغيّر أي عقدة.',
   'flows.copilot.accept': 'تطبيق على المسودة',
+  'flows.copilot.acceptAndSave': 'قبول وحفظ',
+  'flows.copilot.saving': 'جارٍ الحفظ…',
   'flows.copilot.reject': 'تجاهل',
   'flows.copilot.previewHint': 'جارٍ مراجعة مسودة مقترحة: لم يُحفظ شيء بعد.',
   'flows.copilot.repairDisplay': 'فشل تشغيل؛ راجعه واقترح إصلاحًا.',

app/src/lib/i18n/bn.ts

@@ -4114,6 +4114,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count}টি সরানো হয়েছে',
   'flows.copilot.noChanges': 'এই প্রস্তাব কোনো নোড পরিবর্তন করে না।',
   'flows.copilot.accept': 'খসড়ায় প্রয়োগ করুন',
+  'flows.copilot.acceptAndSave': 'গ্রহণ ও সংরক্ষণ করুন',
+  'flows.copilot.saving': 'সংরক্ষণ করা হচ্ছে…',
   'flows.copilot.reject': 'বাতিল করুন',
   'flows.copilot.previewHint':
     'একটি প্রস্তাবিত খসড়া পর্যালোচনা হচ্ছে: এখনও কিছু সংরক্ষণ করা হয়নি।',

app/src/lib/i18n/de.ts

@@ -4229,6 +4229,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} entfernt',
   'flows.copilot.noChanges': 'Dieser Vorschlag ändert keine Knoten.',
   'flows.copilot.accept': 'Auf Entwurf anwenden',
+  'flows.copilot.acceptAndSave': 'Übernehmen & speichern',
+  'flows.copilot.saving': 'Wird gespeichert…',
   'flows.copilot.reject': 'Verwerfen',
   'flows.copilot.previewHint':
     'Ein vorgeschlagener Entwurf wird geprüft: es wurde noch nichts gespeichert.',

app/src/lib/i18n/en.ts

@@ -4804,6 +4804,8 @@ const en: TranslationMap = {
   'flows.copilot.removed': '{count} removed',
   'flows.copilot.noChanges': 'No node changes in this proposal.',
   'flows.copilot.accept': 'Apply to draft',
+  'flows.copilot.acceptAndSave': 'Accept & save',
+  'flows.copilot.saving': 'Saving…',
   'flows.copilot.reject': 'Dismiss',
   'flows.copilot.previewHint': 'Reviewing a proposed draft: nothing is saved yet.',
   'flows.copilot.repairDisplay': 'A run failed: please look at it and propose a fix.',

app/src/lib/i18n/es.ts

@@ -4185,6 +4185,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} eliminados',
   'flows.copilot.noChanges': 'Esta propuesta no cambia ningún nodo.',
   'flows.copilot.accept': 'Aplicar al borrador',
+  'flows.copilot.acceptAndSave': 'Aceptar y guardar',
+  'flows.copilot.saving': 'Guardando…',
   '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.',

app/src/lib/i18n/fr.ts

@@ -4214,6 +4214,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} supprimés',
   'flows.copilot.noChanges': 'Cette proposition ne modifie aucun nœud.',
   'flows.copilot.accept': 'Appliquer au brouillon',
+  'flows.copilot.acceptAndSave': 'Accepter et enregistrer',
+  'flows.copilot.saving': 'Enregistrement…',
   '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.',

app/src/lib/i18n/hi.ts

@@ -4112,6 +4112,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} हटाए गए',
   'flows.copilot.noChanges': 'यह प्रस्ताव किसी नोड को नहीं बदलता।',
   'flows.copilot.accept': 'ड्राफ़्ट पर लागू करें',
+  'flows.copilot.acceptAndSave': 'स्वीकार करें और सहेजें',
+  'flows.copilot.saving': 'सहेजा जा रहा है…',
   'flows.copilot.reject': 'रद्द करें',
   'flows.copilot.previewHint':
     'एक प्रस्तावित ड्राफ़्ट की समीक्षा हो रही है: अभी कुछ सहेजा नहीं गया।',

app/src/lib/i18n/id.ts

@@ -4129,6 +4129,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} dihapus',
   'flows.copilot.noChanges': 'Usulan ini tidak mengubah simpul apa pun.',
   'flows.copilot.accept': 'Terapkan ke draf',
+  'flows.copilot.acceptAndSave': 'Terima & simpan',
+  'flows.copilot.saving': 'Menyimpan…',
   '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.',

app/src/lib/i18n/it.ts

@@ -4182,6 +4182,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} rimossi',
   'flows.copilot.noChanges': 'Questa proposta non modifica alcun nodo.',
   'flows.copilot.accept': 'Applica alla bozza',
+  'flows.copilot.acceptAndSave': 'Accetta e salva',
+  'flows.copilot.saving': 'Salvataggio…',
   '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.',

app/src/lib/i18n/ko.ts

@@ -4067,6 +4067,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count}개 제거',
   'flows.copilot.noChanges': '이 제안은 노드를 변경하지 않습니다.',
   'flows.copilot.accept': '초안에 적용',
+  'flows.copilot.acceptAndSave': '수락 및 저장',
+  'flows.copilot.saving': '저장 중…',
   'flows.copilot.reject': '버리기',
   'flows.copilot.previewHint': '제안된 초안을 검토 중입니다: 아직 저장되지 않았습니다.',
   'flows.copilot.repairDisplay': '실행이 실패했습니다. 확인하고 수정을 제안하세요.',

app/src/lib/i18n/pl.ts

@@ -4166,6 +4166,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': 'usunięto: {count}',
   '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.saving': 'Zapisywanie…',
   'flows.copilot.reject': 'Odrzuć',
   'flows.copilot.previewHint':
     'Przeglądasz proponowaną wersję roboczą: nic nie zostało jeszcze zapisane.',

app/src/lib/i18n/pt.ts

@@ -4174,6 +4174,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '{count} removidos',
   'flows.copilot.noChanges': 'Esta proposta não altera nenhum nó.',
   'flows.copilot.accept': 'Aplicar ao rascunho',
+  'flows.copilot.acceptAndSave': 'Aceitar e salvar',
+  'flows.copilot.saving': 'Salvando…',
   '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.',

app/src/lib/i18n/ru.ts

@@ -4111,7 +4111,7 @@ const messages: TranslationMap = {
   'flows.promptBar.submit': 'Создать',
   'flows.promptBar.startBuilding': 'Начать создание',
   'flows.promptBar.disclaimer':
-    'Копайлот — это ИИ, и он может ошибаться. Пожалуйста, проверяйте ответы.',
+    'Копайлот работает на основе ИИ и может ошибаться. Пожалуйста, проверяйте ответы.',
   'flows.promptBar.thinking': 'Создание…',
   'flows.promptBar.heroTitle': 'Опишите рабочий процесс',
   'flows.promptBar.heroSubtitle':
@@ -4152,6 +4152,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': 'удалено: {count}',
   'flows.copilot.noChanges': 'Это предложение не меняет ни одного узла.',
   'flows.copilot.accept': 'Применить к черновику',
+  'flows.copilot.acceptAndSave': 'Принять и сохранить',
+  'flows.copilot.saving': 'Сохранение…',
   'flows.copilot.reject': 'Отклонить',
   'flows.copilot.previewHint': 'Просмотр предложенного черновика: пока ничего не сохранено.',
   'flows.copilot.repairDisplay': 'Запуск завершился ошибкой; изучите его и предложите исправление.',

app/src/lib/i18n/zh-CN.ts

@@ -3896,6 +3896,8 @@ const messages: TranslationMap = {
   'flows.copilot.removed': '移除 {count} 个',
   'flows.copilot.noChanges': '此方案未更改任何节点。',
   'flows.copilot.accept': '应用到草稿',
+  'flows.copilot.acceptAndSave': '接受并保存',
+  'flows.copilot.saving': '保存中…',
   'flows.copilot.reject': '放弃',
   'flows.copilot.previewHint': '正在查看建议的草稿：尚未保存任何内容。',
   'flows.copilot.repairDisplay': '一次运行失败了，请查看并提出修复方案。',

app/src/pages/FlowCanvasPage.tsx

@@ -418,6 +418,95 @@ function FlowEditor({
   // it survives any number of accept/reject/preview remounts.
   const persistedGraphRef = useRef<WorkflowGraph>(graph);
 
+  // Persist the live graph. A saved flow updates in place via `flows_update`; a
+  // draft is created via `flows_create` (the single persistence gate — an
+  // agent's `propose_workflow` never reaches this RPC), then we replace into
+  // the new flow's canonical `/flows/:id` canvas so further saves update it.
+  // Rejections propagate so the canvas surfaces the failure inline (and leaves
+  // the draft dirty).
+  //
+  // Issue B21: `flows_update` re-validates/normalizes the graph server-side
+  // (schema migration, id defaults, port normalization, etc.) before
+  // persisting, so the canonical saved shape can differ from what the client
+  // sent. Re-sync the canvas draft from the RESPONSE (not just the just-sent
+  // `next`) and bump `canvasVersion` so the editable canvas re-seeds from the
+  // canonical persisted graph immediately — matching what a navigate-away-
+  // and-back remount would show, without requiring one.
+  //
+  // Declared ahead of `handleAcceptProposal` (below), which calls it directly
+  // to persist an accepted proposal immediately.
+  const handleSave = useCallback(
+    async (next: WorkflowGraph, overrideName?: string, overrideRequireApproval?: 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
+      // state updates don't land until the next render. Falling back to the
+      // (possibly stale) `name` keeps the normal manual-Save call site
+      // (which never passes an override) unaffected.
+      const effectiveName = overrideName ?? name;
+      // Same stale-closure concern for `requireApproval`: an accepted
+      // proposal carries its own approval policy (`WorkflowProposal.
+      // requireApproval`), which must win over the currently-loaded canvas
+      // policy — otherwise Accept would silently keep the old flow's policy
+      // instead of the one the agent proposed. A plain manual Save never
+      // passes an override, so `requireApproval` (the loaded flow's current
+      // policy) is unaffected.
+      const effectiveRequireApproval = overrideRequireApproval ?? requireApproval;
+      if (isDraft) {
+        log(
+          'save: creating draft name=%s nodes=%d edges=%d requireApproval=%s',
+          effectiveName,
+          next.nodes.length,
+          next.edges.length,
+          effectiveRequireApproval
+        );
+        const created = await createFlow(effectiveName, next, effectiveRequireApproval);
+        log('save: draft persisted as flow id=%s', created.id);
+        navigate(`/flows/${created.id}`, { replace: true });
+        return;
+      }
+      // Only include `name` / `requireApproval` in the update payload when
+      // they actually diverge from what's already persisted (a manual
+      // rename already persisted `name` via `commitRename`; a copilot-
+      // adopted placeholder name or proposal-driven policy has not) — keeps
+      // the update metadata-safe and avoids needless writes.
+      const nameChanged = effectiveName !== persistedNameRef.current;
+      const requireApprovalChanged = overrideRequireApproval !== undefined;
+      log(
+        'save: flow id=%s nodes=%d edges=%d nameChanged=%s requireApprovalChanged=%s',
+        flowId,
+        next.nodes.length,
+        next.edges.length,
+        nameChanged,
+        requireApprovalChanged
+      );
+      const updated = await updateFlow(flowId, {
+        graph: next,
+        ...(nameChanged ? { name: effectiveName } : {}),
+        ...(requireApprovalChanged ? { requireApproval: effectiveRequireApproval } : {}),
+      });
+      const persisted = updated.graph as WorkflowGraph;
+      persistedGraphRef.current = persisted;
+      persistedNameRef.current = updated.name;
+      setDraftGraph(persisted);
+      if (updated.name !== name) {
+        // Re-sync BOTH title states from the response — leaving `titleDraft`
+        // stale would show the pre-save value in the input and could
+        // resubmit it verbatim on a later blur.
+        setName(updated.name);
+        setTitleDraft(updated.name);
+      }
+      setCanvasVersion(v => v + 1);
+      log(
+        'save: flow id=%s persisted — canvas re-synced from response nodes=%d edges=%d',
+        flowId,
+        persisted.nodes.length,
+        persisted.edges.length
+      );
+    },
+    [isDraft, flowId, name, requireApproval, navigate]
+  );
+
   const handleGraphChange = useCallback(
     (next: WorkflowGraph) => {
       // Freeze the draft while a proposal is under review — the preview graph
@@ -444,18 +533,28 @@ function FlowEditor({
     [draftGraph]
   );
 
+  // Accept now REVIEWS + SAVES in one step: the canvas copilot's own inline
+  // proposal card previously only applied the proposal to the local
+  // `draftGraph` and left persistence to a separate header Save click the
+  // user rarely noticed — the proposal would look "accepted" while nothing
+  // was actually saved (confirmed live: a flow persisted as empty
+  // trigger-only after the user said "looks good"/"save it" and the agent
+  // had no further action to take). Accept now immediately persists the
+  // just-applied graph via `handleSave`, matching what a manual Save click
+  // 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.
   const handleAcceptProposal = useCallback(
-    (proposal: WorkflowProposal) => {
+    async (proposal: WorkflowProposal) => {
       log('copilot proposal accepted');
-      setDraftGraph(proposal.graph as WorkflowGraph);
+      const proposedGraph = proposal.graph as WorkflowGraph;
+      setDraftGraph(proposedGraph);
       setPreview(null);
       setCanvasVersion(v => v + 1);
 
       // Adopt the proposal's name into the flow title, but ONLY while the
       // title is still the generic placeholder — never clobber a user-chosen
-      // or description-derived meaningful name. This does not persist by
-      // itself (matching the "accept doesn't persist" invariant below) — it
-      // rides into the next Save via `name`/`handleSave`.
+      // or description-derived meaningful name.
       //
       // Check the VISIBLE `titleDraft`, not the committed `name` — `name`
       // only updates on blur/Enter via `commitRename`, so if the user is
@@ -466,6 +565,11 @@ function FlowEditor({
       // entirely while `renaming` is true — an in-flight `commitRename`
       // persist must not race with a local proposal-driven rename.
       const proposedName = proposal.name?.trim();
+      // `handleSave` closes over `name`, which — even after `setName` below —
+      // won't reflect the adopted name until the NEXT render (stale-closure
+      // pitfall). Pass it through explicitly as an override instead of
+      // relying on `name` to have updated in time.
+      let overrideName: string | undefined;
       if (
         proposedName &&
         !renaming &&
@@ -478,9 +582,28 @@ function FlowEditor({
         );
         setName(proposedName);
         setTitleDraft(proposedName);
+        overrideName = proposedName;
+      }
+
+      // Persist immediately — this is the actual fix. Do NOT route through
+      // `canvasRef.current?.save()`: the canvas is mid-remount (the
+      // `canvasVersion` bump above) so the ref's imperative handle is stale;
+      // call `handleSave` directly with the known-good proposed graph.
+      try {
+        await handleSave(proposedGraph, overrideName, proposal.requireApproval);
+        log('copilot proposal accepted: persisted');
+      } catch (err) {
+        log('copilot proposal accepted: save 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`.
+        throw err;
       }
     },
-    [titleDraft, renaming, t, isDraft]
+    [titleDraft, renaming, t, isDraft, handleSave]
   );
 
   const handleRejectProposal = useCallback(() => {
@@ -535,69 +658,6 @@ function FlowEditor({
     [initialCopilotSeed]
   );
 
-  // Persist the live graph. A saved flow updates in place via `flows_update`; a
-  // draft is created via `flows_create` (the single persistence gate — an
-  // agent's `propose_workflow` never reaches this RPC), then we replace into
-  // the new flow's canonical `/flows/:id` canvas so further saves update it.
-  // Rejections propagate so the canvas surfaces the failure inline (and leaves
-  // the draft dirty).
-  //
-  // Issue B21: `flows_update` re-validates/normalizes the graph server-side
-  // (schema migration, id defaults, port normalization, etc.) before
-  // persisting, so the canonical saved shape can differ from what the client
-  // sent. Re-sync the canvas draft from the RESPONSE (not just the just-sent
-  // `next`) and bump `canvasVersion` so the editable canvas re-seeds from the
-  // canonical persisted graph immediately — matching what a navigate-away-
-  // and-back remount would show, without requiring one.
-  const handleSave = useCallback(
-    async (next: WorkflowGraph) => {
-      if (isDraft) {
-        log(
-          'save: creating draft name=%s nodes=%d edges=%d',
-          name,
-          next.nodes.length,
-          next.edges.length
-        );
-        const created = await createFlow(name, next, requireApproval);
-        log('save: draft persisted as flow id=%s', created.id);
-        navigate(`/flows/${created.id}`, { replace: true });
-        return;
-      }
-      // Only include `name` in the update payload when it actually diverges
-      // from the last-known-persisted baseline (a manual rename already
-      // persisted it via `commitRename`; a copilot-adopted placeholder name
-      // has not) — keeps the update metadata-safe and avoids needless renames.
-      const nameChanged = name !== persistedNameRef.current;
-      log(
-        'save: flow id=%s nodes=%d edges=%d nameChanged=%s',
-        flowId,
-        next.nodes.length,
-        next.edges.length,
-        nameChanged
-      );
-      const updated = await updateFlow(flowId, { graph: next, ...(nameChanged ? { name } : {}) });
-      const persisted = updated.graph as WorkflowGraph;
-      persistedGraphRef.current = persisted;
-      persistedNameRef.current = updated.name;
-      setDraftGraph(persisted);
-      if (updated.name !== name) {
-        // Re-sync BOTH title states from the response — leaving `titleDraft`
-        // stale would show the pre-save value in the input and could
-        // resubmit it verbatim on a later blur.
-        setName(updated.name);
-        setTitleDraft(updated.name);
-      }
-      setCanvasVersion(v => v + 1);
-      log(
-        'save: flow id=%s persisted — canvas re-synced from response nodes=%d edges=%d',
-        flowId,
-        persisted.nodes.length,
-        persisted.edges.length
-      );
-    },
-    [isDraft, flowId, name, requireApproval, navigate]
-  );
-
   // Warn on hard tab close / reload while there are unsaved edits.
   useEffect(() => {
     if (!dirty) return;

app/src/pages/__tests__/FlowCanvasPage.test.tsx

@@ -454,6 +454,12 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
     listFlowConnections.mockReset();
     validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] });
     listFlowConnections.mockResolvedValue([]);
+    // Accept now persists immediately (review + save in one step, see
+    // `handleAcceptProposal`) — default every test to a successful save so
+    // tests that only care about the title/name adoption aren't tripped up
+    // by an unmocked (`undefined`-resolving) `updateFlow`/`createFlow`.
+    updateFlow.mockResolvedValue(makeFlow());
+    createFlow.mockResolvedValue(makeFlow({ id: 'created-id' }));
   });
 
   function renderEditor(id = 'test-id') {
@@ -467,47 +473,51 @@ 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()) {
+    return act(async () => {
+      await (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => Promise<void>)(
+        proposal
+      );
+    });
+  }
+
   it('adopts the proposal name when the title is the generic placeholder', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: 'New workflow' }));
+    updateFlow.mockResolvedValue(makeFlow({ name: 'Standup reminder' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
     expect(screen.getByTestId('flow-canvas-title')).toHaveValue('New workflow');
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
+    await acceptProposal();
 
-    await waitFor(() =>
-      expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
-    );
+    expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder');
   });
 
   it('adopts the proposal name when the title is blank', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: '' }));
+    updateFlow.mockResolvedValue(makeFlow({ name: 'Standup reminder' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
+    await acceptProposal();
 
-    await waitFor(() =>
-      expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
-    );
+    expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder');
   });
 
   it('does not clobber a user-set title when accepting a proposal', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: 'My flow' }));
+    // The name is unchanged by this accept, so the accept-triggered save's
+    // response echoes it back unchanged too — matching a real server, which
+    // only touches `name` when it's part of the update payload.
+    updateFlow.mockResolvedValue(makeFlow({ name: 'My flow' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
+    await acceptProposal();
 
-    // Give any (incorrect) state update a chance to flush, then assert the
-    // title is unchanged.
-    await Promise.resolve();
     expect(screen.getByTestId('flow-canvas-title')).toHaveValue('My flow');
   });
 
@@ -518,6 +528,9 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
   // VISIBLE `titleDraft`, or it clobbers in-progress typing.
   it('does not clobber an in-progress (uncommitted) title edit when accepting a proposal', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: 'New workflow' }));
+    // The uncommitted edit never reaches `name`, so the accept-triggered
+    // save's payload/response name is unchanged from the loaded flow.
+    updateFlow.mockResolvedValue(makeFlow({ name: 'New workflow' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
@@ -527,67 +540,53 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
     });
     expect(screen.getByTestId('flow-canvas-title')).toHaveValue('My in-progress title');
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
+    await acceptProposal();
 
-    await Promise.resolve();
     expect(screen.getByTestId('flow-canvas-title')).toHaveValue('My in-progress title');
   });
 
-  it('includes the adopted name in the flows_update payload on Save (persisted flow)', async () => {
+  it('Accept on an existing flow fires updateFlow(flowId, { graph, name }) immediately, with no separate Save click', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: 'New workflow' }));
     updateFlow.mockResolvedValue(makeFlow({ name: 'Standup reminder' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
-    await waitFor(() =>
-      expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
-    );
+    await acceptProposal();
 
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
-    fireEvent.click(screen.getByTestId('flow-action-confirm-accept'));
-
-    await waitFor(() => expect(updateFlow).toHaveBeenCalledTimes(1));
+    expect(updateFlow).toHaveBeenCalledTimes(1);
     const [calledId, update] = updateFlow.mock.calls[0];
     expect(calledId).toBe('test-id');
     expect(update.name).toBe('Standup reminder');
     expect(update.graph).toBeDefined();
+    // The accept-triggered save re-syncs the canvas from the response and
+    // clears the dirty baseline — no lingering "Unsaved changes" badge, and
+    // no manual Save click was fired to get here.
+    expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument();
   });
 
   // Regression (CodeRabbit on #4886): accepting a proposal that changes only
   // the top-level `name` (graph unchanged) previously left the editor's dirty
   // state false — since the graph-only diff saw no change — so Save stayed
-  // disabled and the adopted title could never be persisted.
-  it('marks the editor dirty when an accepted proposal changes only the name', async () => {
+  // disabled and the adopted title could never be persisted. Accept now saves
+  // immediately regardless of dirty tracking, so this asserts the
+  // accept-triggered save still fires — and carries the adopted name — even
+  // when the graph itself didn't change.
+  it('persists a name-only accepted proposal (graph unchanged) via the accept-triggered save', async () => {
     const flow = makeFlow({ name: 'New workflow' });
     getFlow.mockResolvedValue(flow);
+    updateFlow.mockResolvedValue(makeFlow({ name: 'Standup reminder' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
     // Clean on load.
     expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument();
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(
-        makeProposal({ name: 'Standup reminder', graph: flow.graph })
-      );
-    });
+    await acceptProposal(makeProposal({ name: 'Standup reminder', graph: flow.graph }));
 
     await waitFor(() =>
       expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
     );
-    // Graph is byte-identical to the persisted baseline — only the name
-    // changed — but the editor must still report dirty so Save is enabled.
-    await waitFor(() => expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument());
-
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
-    fireEvent.click(screen.getByTestId('flow-action-confirm-accept'));
-
-    await waitFor(() => expect(updateFlow).toHaveBeenCalledTimes(1));
+    expect(updateFlow).toHaveBeenCalledTimes(1);
     const [, update] = updateFlow.mock.calls[0];
     expect(update.name).toBe('Standup reminder');
   });
@@ -596,30 +595,35 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
   // differs from what was submitted (server-side normalization), the title
   // input must re-sync to the persisted value too — not just the committed
   // `name` — or the stale draft can be resubmitted verbatim on a later blur.
-  it('re-syncs titleDraft from the persisted response name on Save', async () => {
+  // Covers F2: `handleSave` re-syncing both `name` and `titleDraft` from the
+  // server response name.
+  it('re-syncs titleDraft from the persisted response name on the accept-triggered save', async () => {
     getFlow.mockResolvedValue(makeFlow({ name: 'New workflow' }));
     updateFlow.mockResolvedValue(makeFlow({ name: 'Standup Reminder (normalized)' }));
     renderEditor();
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
-    await waitFor(() =>
-      expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
-    );
+    await acceptProposal();
 
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
-    fireEvent.click(screen.getByTestId('flow-action-confirm-accept'));
-
-    await waitFor(() => expect(updateFlow).toHaveBeenCalledTimes(1));
+    // The visible input (`titleDraft`) must reflect the server-normalized
+    // name, not just the committed `name` — otherwise a later blur would
+    // resubmit the stale, pre-normalization value.
     await waitFor(() =>
       expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup Reminder (normalized)')
     );
+    expect(updateFlow).toHaveBeenCalledTimes(1);
   });
 
-  it('passes the adopted name to createFlow on Save (draft flow)', async () => {
+  // Regression for the CodeRabbit finding: the accepted PROPOSAL's own
+  // `requireApproval` policy must reach `createFlow`, not the draft route's
+  // pre-existing value — otherwise Accept would silently keep the old canvas
+  // policy instead of the one the agent proposed. Route state deliberately
+  // uses the OPPOSITE value from the proposal so a test that reads the
+  // route's value (the pre-fix bug) fails loudly instead of passing by
+  // coincidence.
+  it('Accept on a draft canvas fires createFlow(name, graph, requireApproval) using the PROPOSAL policy and navigates to /flows/<id>', async () => {
     createFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' }));
+    getFlow.mockResolvedValue(makeFlow({ id: 'created-id', name: 'Standup reminder' }));
     render(
       <MemoryRouter
         initialEntries={[
@@ -642,6 +646,9 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
                 ],
                 edges: [],
               },
+              // Route (pre-existing draft) policy is FALSE — the opposite of
+              // the proposal below — so the assertion can't pass by both
+              // values coincidentally matching.
               requireApproval: false,
             },
           },
@@ -655,20 +662,92 @@ describe('FlowCanvasPage copilot proposal name adoption', () => {
     await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
     expect(screen.getByTestId('flow-canvas-title')).toHaveValue('New workflow');
 
-    act(() => {
-      (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => void)(makeProposal());
-    });
-    await waitFor(() =>
-      expect(screen.getByTestId('flow-canvas-title')).toHaveValue('Standup reminder')
-    );
+    // Proposal policy is TRUE — must be what reaches `createFlow`.
+    await acceptProposal(makeProposal({ requireApproval: true }));
 
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
-    fireEvent.click(screen.getByTestId('flow-action-confirm-accept'));
-
-    await waitFor(() => expect(createFlow).toHaveBeenCalledTimes(1));
-    const [name] = createFlow.mock.calls[0];
+    expect(createFlow).toHaveBeenCalledTimes(1);
+    const [name, graph, requireApproval] = createFlow.mock.calls[0];
     expect(name).toBe('Standup reminder');
+    expect(graph).toBeDefined();
+    expect(requireApproval).toBe(true);
     expect(updateFlow).not.toHaveBeenCalled();
+
+    // `handleSave`'s draft-create path replaces into the new flow's canonical
+    // route — Accept alone drives that navigation, matching what a manual
+    // Save click right after Accept used to require.
+    await waitFor(() => expect(getFlow).toHaveBeenCalledWith('created-id'));
+  });
+
+  it('Accept on a saved flow fires updateFlow with the PROPOSAL requireApproval policy', async () => {
+    // The loaded flow's persisted policy is FALSE; the accepted proposal's
+    // is TRUE — the update payload must carry the proposal's value, not
+    // silently keep the flow's current one.
+    getFlow.mockResolvedValue(makeFlow({ require_approval: false }));
+    updateFlow.mockResolvedValue(makeFlow({ require_approval: true }));
+    renderEditor();
+    await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
+
+    await acceptProposal(makeProposal({ requireApproval: true }));
+
+    expect(updateFlow).toHaveBeenCalledTimes(1);
+    expect(updateFlow).toHaveBeenCalledWith(
+      'test-id',
+      expect.objectContaining({ requireApproval: true })
+    );
+  });
+
+  // Regression test for the review finding (F1, HIGH): `handleAcceptProposal`
+  // used to swallow a failed accept-triggered save (log + no rethrow), so
+  // `onAccept` always resolved — the copilot panel's `clearProposal()` always
+  // ran, and the proposal card silently vanished on a real save failure with
+  // no way to retry (its own catch branch was dead code). This test fails
+  // without the rethrow (the promise resolves instead of rejecting) and
+  // passes with it.
+  it('rethrows an accept-triggered save failure so the caller can leave the proposal visible for retry', async () => {
+    getFlow.mockResolvedValue(makeFlow({ name: 'My flow' }));
+    updateFlow.mockRejectedValue(new Error('network unreachable'));
+    renderEditor();
+    await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
+    expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument();
+
+    // Catch INSIDE `act()` (rather than via `expect(...).rejects`, which lets
+    // the rejection escape the `act()` scope unhandled) so React still
+    // flushes the synchronous draft/preview updates `handleAcceptProposal`
+    // makes before the failed `await handleSave(...)` — otherwise the
+    // assertions below would race an incomplete render.
+    let caughtErr: unknown;
+    await act(async () => {
+      try {
+        await (copilotPanelProps.current?.onAccept as (p: WorkflowProposal) => Promise<void>)(
+          makeProposal()
+        );
+      } catch (err) {
+        caughtErr = err;
+      }
+    });
+
+    // This is the fix under test: without the rethrow, `caughtErr` stays
+    // `undefined` and this assertion fails.
+    expect(caughtErr).toBeInstanceOf(Error);
+    expect((caughtErr as Error).message).toBe('network unreachable');
+
+    // The draft is already applied before `handleSave` is even attempted, so
+    // rethrowing loses no data: the proposal's graph is still on the canvas
+    // (2 nodes: the original trigger + the proposal's agent node), dirty,
+    // with the header Save button enabled as the manual retry — matching
+    // what `WorkflowCopilotPanel`'s own catch branch (which skips
+    // `clearProposal()` on rejection) relies on to keep the card visible.
+    //
+    // These three assertions land on state derived from the REMOUNTED canvas
+    // (`handleAcceptProposal` bumps `canvasVersion`, which changes the
+    // `<FlowCanvas key=...>` and forces a fresh child mount) — its own
+    // mount-time effects (`onDirtyChange`/`onSaveMetaChange`) can settle on a
+    // later microtask/effect flush than the outer `act()` above guarantees,
+    // so poll via `waitFor` instead of asserting immediately (this was
+    // observed to occasionally race in CI).
+    await waitFor(() => expect(screen.getAllByTestId('flow-node')).toHaveLength(2));
+    await waitFor(() => expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument());
+    await waitFor(() => expect(screen.getByTestId('flow-editor-save')).not.toBeDisabled());
   });
 });