diff --git a/app/src/components/chat/WorkflowProposalCard.test.tsx b/app/src/components/chat/WorkflowProposalCard.test.tsx index bb2179da9..83f79fe6b 100644 --- a/app/src/components/chat/WorkflowProposalCard.test.tsx +++ b/app/src/components/chat/WorkflowProposalCard.test.tsx @@ -13,15 +13,18 @@ vi.mock('../../lib/i18n/I18nContext', () => ({ useT: () => ({ t: (key: string) = // time the (hoisted) factory runs. (These specific names happened to work // without it, since Vitest's compiler special-cases `mock`-prefixed // identifiers, but that's an incidental heuristic, not a guarantee.) -const { mockCreateFlow, mockUpdateFlow, mockDispatch, mockNavigate } = vi.hoisted(() => ({ - mockCreateFlow: vi.fn(), - mockUpdateFlow: vi.fn(), - mockDispatch: vi.fn(), - mockNavigate: vi.fn(), -})); +const { mockCreateFlow, mockUpdateFlow, mockSetFlowEnabled, mockDispatch, mockNavigate } = + vi.hoisted(() => ({ + mockCreateFlow: vi.fn(), + mockUpdateFlow: vi.fn(), + mockSetFlowEnabled: vi.fn(), + mockDispatch: vi.fn(), + mockNavigate: vi.fn(), + })); vi.mock('../../services/api/flowsApi', () => ({ createFlow: (...args: unknown[]) => mockCreateFlow(...args), updateFlow: (...args: unknown[]) => mockUpdateFlow(...args), + setFlowEnabled: (...args: unknown[]) => mockSetFlowEnabled(...args), })); vi.mock('../../store/hooks', () => ({ useAppDispatch: () => mockDispatch })); vi.mock('react-router-dom', () => ({ useNavigate: () => mockNavigate })); @@ -44,8 +47,11 @@ function proposal(partial: Partial = {}): WorkflowProposal { describe('WorkflowProposalCard', () => { beforeEach(() => { - mockCreateFlow.mockReset().mockResolvedValue({ id: 'f1', name: 'Daily standup summary' }); + mockCreateFlow + .mockReset() + .mockResolvedValue({ id: 'f1', name: 'Daily standup summary', enabled: true }); mockUpdateFlow.mockReset(); + mockSetFlowEnabled.mockReset().mockResolvedValue({ id: 'f1', enabled: true }); mockDispatch.mockReset(); mockNavigate.mockReset(); }); @@ -74,6 +80,8 @@ describe('WorkflowProposalCard', () => { expect(mockCreateFlow).toHaveBeenCalledWith(p.name, p.graph, p.requireApproval) ); expect(mockDispatch).toHaveBeenCalledTimes(1); + // createFlow already came back enabled — no need for a follow-up arm. + expect(mockSetFlowEnabled).not.toHaveBeenCalled(); }); it('shows a loading state while saving', async () => { @@ -86,7 +94,7 @@ describe('WorkflowProposalCard', () => { render(); fireEvent.click(screen.getByText('chat.flowProposal.save')); await waitFor(() => expect(screen.getByText('chat.flowProposal.saving')).toBeInTheDocument()); - resolveCreate({ id: 'f1' }); + resolveCreate({ id: 'f1', enabled: true }); }); it('surfaces an error and stays mounted when createFlow fails', async () => { @@ -96,6 +104,51 @@ describe('WorkflowProposalCard', () => { await waitFor(() => expect(screen.getByText(/chat\.flowProposal\.error/)).toBeInTheDocument()); // Not cleared on failure. expect(mockDispatch).not.toHaveBeenCalled(); + expect(mockSetFlowEnabled).not.toHaveBeenCalled(); + }); + + // Issue B29 Rule 1: an automatic-trigger graph (schedule/app_event/webhook) + // comes back from `flows_create` disabled, regardless of what the caller + // asked for. "Save & enable" is the user's own explicit arming click, so + // the card must follow up with `setFlowEnabled` to actually arm it — + // otherwise the CTA's own label would be a lie and the flow would never + // fire despite the user clicking "enable". + it('explicitly enables via setFlowEnabled when createFlow returns a disabled auto-trigger flow', async () => { + mockCreateFlow.mockResolvedValueOnce({ + id: 'f1', + name: 'Daily standup summary', + enabled: false, + }); + const p = proposal(); + render(); + fireEvent.click(screen.getByText('chat.flowProposal.save')); + await waitFor(() => expect(mockSetFlowEnabled).toHaveBeenCalledWith('f1', true)); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); + + it('keeps the flow saved and lets the user retry just the enable step if setFlowEnabled fails', async () => { + mockCreateFlow.mockResolvedValueOnce({ + id: 'f1', + name: 'Daily standup summary', + enabled: false, + }); + mockSetFlowEnabled.mockRejectedValueOnce(new Error('enable boom')); + render(); + fireEvent.click(screen.getByText('chat.flowProposal.save')); + await waitFor(() => + expect(screen.getByText(/chat\.flowProposal\.enableError/)).toBeInTheDocument() + ); + // The flow was already persisted — the card must not be cleared, and a + // retry must not re-create it (which would duplicate the flow). + expect(mockDispatch).not.toHaveBeenCalled(); + + mockSetFlowEnabled.mockResolvedValueOnce({ id: 'f1', enabled: true }); + fireEvent.click(screen.getByText('chat.flowProposal.save')); + await waitFor(() => expect(mockDispatch).toHaveBeenCalledTimes(1)); + // Only ever created once, even though "Save & enable" was clicked twice. + expect(mockCreateFlow).toHaveBeenCalledTimes(1); + expect(mockSetFlowEnabled).toHaveBeenCalledTimes(2); + expect(mockSetFlowEnabled).toHaveBeenLastCalledWith('f1', true); }); it('opens the proposed graph in the canvas as an unsaved draft without persisting', () => { diff --git a/app/src/components/chat/WorkflowProposalCard.tsx b/app/src/components/chat/WorkflowProposalCard.tsx index 7db034f3b..37e66c4b6 100644 --- a/app/src/components/chat/WorkflowProposalCard.tsx +++ b/app/src/components/chat/WorkflowProposalCard.tsx @@ -5,7 +5,7 @@ import { useNavigate } from 'react-router-dom'; import { FLOW_CANVAS_DRAFT_ROUTE, type FlowCanvasDraftState } from '../../lib/flows/canvasDraft'; import type { WorkflowGraph } from '../../lib/flows/types'; import { useT } from '../../lib/i18n/I18nContext'; -import { createFlow } from '../../services/api/flowsApi'; +import { createFlow, setFlowEnabled } from '../../services/api/flowsApi'; import { clearWorkflowProposalForThread, type WorkflowProposal, @@ -36,6 +36,18 @@ interface Props { * the client; the agent has no way to reach that RPC on its own. "Dismiss" * just clears the proposal without saving anything. * + * B29 (save/enable safety) Rule 1 forces `flows_create` to persist an + * automatic-trigger graph (`schedule` / `app_event` / `webhook`) DISABLED, + * no matter what the caller passed — that's what stops a copilot + * `save_workflow` autosave from silently arming an unattended automation. + * But "Save & enable" is the user's own explicit arming click, not a silent + * autosave, so when `createFlow` hands back a disabled flow here, `save` + * follows up with an explicit {@link setFlowEnabled} call — the same toggle + * `flows_set_enabled` exposes everywhere else — so the button does what it + * says. If that follow-up enable call itself fails, the flow stays saved + * (disabled) and the card keeps `savedFlowId` around so a retry re-enables + * instead of re-creating (which would duplicate the flow). + * * Mirrors {@link PlanReviewCard}'s placement/chrome above the composer, and * the tool-timeline `StatusTag`/detail-chip visual language for the * node-kind badges + config hints in the step list. @@ -46,6 +58,11 @@ export const WorkflowProposalCard: React.FC = ({ threadId, proposal, onSa const navigate = useNavigate(); const [saving, setSaving] = useState(false); const [errorMsg, setErrorMsg] = useState(null); + // Set once `createFlow` has persisted the flow but it came back disabled + // (B29 Rule 1) and the follow-up `setFlowEnabled` call hasn't succeeded + // yet. Non-null means a retry of `save` should skip `createFlow` entirely + // and only retry the enable step. + const [savedFlowId, setSavedFlowId] = useState(null); const dismiss = () => { dispatch(clearWorkflowProposalForThread({ threadId })); @@ -80,13 +97,37 @@ export const WorkflowProposalCard: React.FC = ({ threadId, proposal, onSa if (saving) return; setSaving(true); setErrorMsg(null); + // Track persistence locally (not via `savedFlowId` state) because a + // `setState` call doesn't apply synchronously — reading `savedFlowId` + // itself in the `catch` below would see this render's stale value, not + // what just happened in this attempt. + let flowId = savedFlowId; + let flowPersisted = flowId !== null; try { - await createFlow(proposal.name, proposal.graph, proposal.requireApproval); + if (!flowId) { + const flow = await createFlow(proposal.name, proposal.graph, proposal.requireApproval); + flowId = flow.id; + flowPersisted = true; + if (flow.enabled) { + log('save: createFlow returned enabled — nothing further to arm id=%s', flow.id); + dispatch(clearWorkflowProposalForThread({ threadId })); + onSaved?.(); + return; + } + // B29 Rule 1 saved this automatic-trigger flow disabled. This click + // is the user's own explicit "Save & enable" — not the copilot's + // silent autosave Rule 1 guards against — so arm it now. + log('save: createFlow returned disabled (Rule 1) — arming explicitly id=%s', flow.id); + setSavedFlowId(flow.id); + } + await setFlowEnabled(flowId, true); dispatch(clearWorkflowProposalForThread({ threadId })); onSaved?.(); } catch (e) { - log('createFlow failed: %o', e); - setErrorMsg(t('chat.flowProposal.error')); + log('save failed (createFlow/setFlowEnabled): %o', e); + setErrorMsg( + flowPersisted ? t('chat.flowProposal.enableError') : t('chat.flowProposal.error') + ); setSaving(false); } }; diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 9b1bd2812..84739360c 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -3175,6 +3175,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'فتح في اللوحة', 'chat.flowProposal.dismiss': 'إغلاق', 'chat.flowProposal.error': 'تعذّر حفظ سير العمل. حاول مرة أخرى.', + 'chat.flowProposal.enableError': + 'تم حفظ سير العمل، لكن تعذّر تفعيله. حاول مرة أخرى، أو فعّله من صفحة سير العمل.', 'channels.authMode.managed_dm': 'قم بتسجيل الدخول باستخدام OpenHuman', 'channels.authMode.oauth': 'OAuth تسجيل الدخول', 'channels.authMode.bot_token': 'استخدم رمز الروبوت الخاص بك', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index f58c6db71..7bc4b2653 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -3244,6 +3244,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'ক্যানভাসে খুলুন', 'chat.flowProposal.dismiss': 'খারিজ করুন', 'chat.flowProposal.error': 'ওয়ার্কফ্লো সংরক্ষণ করা যায়নি। আবার চেষ্টা করুন।', + 'chat.flowProposal.enableError': + 'ওয়ার্কফ্লো সংরক্ষিত হয়েছে, কিন্তু সক্ষম করা যায়নি। আবার চেষ্টা করুন, বা Workflows পৃষ্ঠা থেকে সক্ষম করুন।', 'channels.authMode.managed_dm': 'OpenHuman দিয়ে লগইন করুন', 'channels.authMode.oauth': 'OAuth সাইন-ইন করুন', 'channels.authMode.bot_token': 'আপনার নিজের বট টোকেন ব্যবহার করুন', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 57d1e6365..1d01da56c 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -3326,6 +3326,8 @@ const messages: TranslationMap = { 'chat.flowProposal.dismiss': 'Verwerfen', 'chat.flowProposal.error': 'Der Workflow konnte nicht gespeichert werden. Bitte versuchen Sie es erneut.', + 'chat.flowProposal.enableError': + 'Workflow gespeichert, konnte aber nicht aktiviert werden. Versuchen Sie es erneut oder aktivieren Sie ihn auf der Workflows-Seite.', 'channels.authMode.managed_dm': 'Mit OpenHuman anmelden', 'channels.authMode.oauth': 'OAuth-Anmeldung', 'channels.authMode.bot_token': 'Eigenen Bot-Token verwenden', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 827763fa5..0706813c4 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -3660,6 +3660,8 @@ const en: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Open in canvas', 'chat.flowProposal.dismiss': 'Dismiss', 'chat.flowProposal.error': 'Could not save the workflow. Please try again.', + 'chat.flowProposal.enableError': + 'Workflow saved, but could not enable it. Try again, or enable it from the Workflows page.', // Auth mode labels 'channels.authMode.managed_dm': 'Login with OpenHuman', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index 7c2845d87..d6f251695 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -3301,6 +3301,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Abrir en el lienzo', 'chat.flowProposal.dismiss': 'Descartar', 'chat.flowProposal.error': 'No se pudo guardar el flujo de trabajo. Inténtalo de nuevo.', + 'chat.flowProposal.enableError': + 'Flujo de trabajo guardado, pero no se pudo activar. Inténtalo de nuevo o actívalo desde la página de Workflows.', 'channels.authMode.managed_dm': 'Iniciar sesión con OpenHuman', 'channels.authMode.oauth': 'OAuth Iniciar sesión', 'channels.authMode.bot_token': 'Utilice su propio token de bot', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index b8df7cac4..bedf360ce 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -3315,6 +3315,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Ouvrir dans le canevas', 'chat.flowProposal.dismiss': 'Ignorer', 'chat.flowProposal.error': "Impossible d'enregistrer le workflow. Veuillez réessayer.", + 'chat.flowProposal.enableError': + "Workflow enregistré, mais impossible de l'activer. Réessayez, ou activez-le depuis la page Workflows.", 'channels.authMode.managed_dm': 'Connectez-vous avec OpenHuman', 'channels.authMode.oauth': 'OAuth Connectez-vous', 'channels.authMode.bot_token': 'Utiliser votre propre jeton de robot', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 0055cb74e..8fe728543 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -3243,6 +3243,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'कैनवास में खोलें', 'chat.flowProposal.dismiss': 'खारिज करें', 'chat.flowProposal.error': 'वर्कफ़्लो सहेजा नहीं जा सका। कृपया फिर से प्रयास करें।', + 'chat.flowProposal.enableError': + 'वर्कफ़्लो सहेजा गया, लेकिन सक्षम नहीं किया जा सका। फिर से प्रयास करें, या Workflows पेज से इसे सक्षम करें।', 'channels.authMode.managed_dm': 'OpenHuman से लॉगिन करें', 'channels.authMode.oauth': 'OAuth साइन-इन करें', 'channels.authMode.bot_token': 'अपने स्वयं के बॉट टोकन का उपयोग करें', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index aca2e57d9..28b94c4ff 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -3256,6 +3256,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Buka di kanvas', 'chat.flowProposal.dismiss': 'Abaikan', 'chat.flowProposal.error': 'Alur kerja tidak dapat disimpan. Silakan coba lagi.', + 'chat.flowProposal.enableError': + 'Alur kerja disimpan, tetapi tidak dapat diaktifkan. Coba lagi, atau aktifkan dari halaman Workflows.', 'channels.authMode.managed_dm': 'Masuk dengan OpenHuman', 'channels.authMode.oauth': 'OAuth Masuk', 'channels.authMode.bot_token': 'Gunakan Token Bot Anda sendiri', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index 04fb95928..3f48adf4c 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -3297,6 +3297,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Apri nel canvas', 'chat.flowProposal.dismiss': 'Ignora', 'chat.flowProposal.error': 'Impossibile salvare il workflow. Riprova.', + 'chat.flowProposal.enableError': + 'Workflow salvato, ma non è stato possibile attivarlo. Riprova, oppure attivalo dalla pagina Workflows.', 'channels.authMode.managed_dm': 'Accedi con OpenHuman', 'channels.authMode.oauth': 'OAuth Accedi', 'channels.authMode.bot_token': 'Usa il tuo token Bot', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index 5fbba2a6c..bf31b95c2 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -3212,6 +3212,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': '캔버스에서 열기', 'chat.flowProposal.dismiss': '닫기', 'chat.flowProposal.error': '워크플로를 저장할 수 없습니다. 다시 시도하세요.', + 'chat.flowProposal.enableError': + '워크플로는 저장되었지만 활성화할 수 없습니다. 다시 시도하거나 Workflows 페이지에서 활성화하세요.', 'channels.authMode.managed_dm': 'OpenHuman로 로그인', 'channels.authMode.oauth': 'OAuth 로그인', 'channels.authMode.bot_token': '자체 봇 토큰 사용', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index 3654662ba..e3d9ddfa8 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -3281,6 +3281,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Otwórz na kanwie', 'chat.flowProposal.dismiss': 'Odrzuć', 'chat.flowProposal.error': 'Nie udało się zapisać przepływu pracy. Spróbuj ponownie.', + 'chat.flowProposal.enableError': + 'Przepływ pracy zapisany, ale nie udało się go włączyć. Spróbuj ponownie lub włącz go na stronie Workflows.', 'channels.authMode.managed_dm': 'Zaloguj się z OpenHuman', 'channels.authMode.oauth': 'Logowanie OAuth', 'channels.authMode.bot_token': 'Użyj własnego tokena bota', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 2c4ebec66..74f57fc2e 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -3299,6 +3299,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Abrir no canvas', 'chat.flowProposal.dismiss': 'Dispensar', 'chat.flowProposal.error': 'Não foi possível salvar o fluxo de trabalho. Tente novamente.', + 'chat.flowProposal.enableError': + 'Fluxo de trabalho salvo, mas não foi possível ativá-lo. Tente novamente ou ative-o na página Workflows.', 'channels.authMode.managed_dm': 'Faça login com OpenHuman', 'channels.authMode.oauth': 'OAuth Faça login', 'channels.authMode.bot_token': 'Use seu próprio token de bot', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 6004f0a58..04741b0f9 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -3271,6 +3271,8 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': 'Открыть на холсте', 'chat.flowProposal.dismiss': 'Скрыть', 'chat.flowProposal.error': 'Не удалось сохранить рабочий процесс. Попробуйте еще раз.', + 'chat.flowProposal.enableError': + 'Рабочий процесс сохранён, но не удалось его включить. Попробуйте ещё раз или включите его на странице Workflows.', 'channels.authMode.managed_dm': 'Войдите с помощью OpenHuman', 'channels.authMode.oauth': 'OAuth Вход в систему', 'channels.authMode.bot_token': 'Используйте свой собственный токен бота', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index fc58c25a0..15300cac4 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -3078,6 +3078,7 @@ const messages: TranslationMap = { 'chat.flowProposal.openInCanvas': '在画布中打开', 'chat.flowProposal.dismiss': '忽略', 'chat.flowProposal.error': '无法保存该工作流。请重试。', + 'chat.flowProposal.enableError': '工作流已保存,但无法启用。请重试,或在“工作流”页面手动启用。', 'channels.authMode.managed_dm': '使用 OpenHuman 登录', 'channels.authMode.oauth': 'OAuth 登录', 'channels.authMode.bot_token': '使用你自己的 Bot Token', diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index ce92b4ef0..27620f159 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -253,6 +253,18 @@ function unwrapCliEnvelope(payload: unknown): T { * `false` when omitted, but the B4 proposal flow always passes it explicitly * (defaulting to `true` on the Rust tool side) so a saved agent-proposed flow * starts with its outbound-action approval gate on. + * + * B29 (save/enable safety) Rule 1: when `graph`'s trigger fires without a + * human in the loop (`schedule` / `app_event` / `webhook`), the server + * ALWAYS persists the flow `enabled: false`, regardless of what the caller + * intended — no creation path may silently hand back an armed, unattended + * automation. The returned {@link Flow}'s `enabled` field reflects this, so + * every caller MUST check it: if the caller represents an explicit + * user-arming action (e.g. `WorkflowProposalCard`'s "Save & enable" click), + * follow up with {@link setFlowEnabled} to actually arm it — that is a + * legitimate explicit enable, not the silent copilot auto-arm Rule 1 guards + * against. A caller with no such explicit intent (e.g. a background/copilot + * save) should leave it disabled and let the user arm it later. */ export async function createFlow( name: string, diff --git a/src/openhuman/flows/builder_tools.rs b/src/openhuman/flows/builder_tools.rs index 7eeb2c6cb..abc3fc338 100644 --- a/src/openhuman/flows/builder_tools.rs +++ b/src/openhuman/flows/builder_tools.rs @@ -55,6 +55,7 @@ use tinyflows::model::WorkflowGraph; use crate::openhuman::config::Config; use crate::openhuman::flows::ops; use crate::openhuman::flows::ops::validate_and_migrate_graph; +use crate::openhuman::flows::tools; use crate::openhuman::security::{AutonomyLevel, SecurityPolicy}; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; @@ -1847,6 +1848,34 @@ impl Tool for SaveWorkflowTool { enabled = flow.enabled, "[flows] save_workflow: persisted" ); + // Issue B29 (save/enable safety), Rule 3: `flows_create` only + // gates the FIRST creation of a flow — an agent `save_workflow` + // targets an EXISTING flow via `flows_update`, which preserves + // whatever `enabled` state the flow already had. If the user + // already armed this flow (enabled it) and it has an automatic + // trigger, saving a new graph onto it re-arms it live with no + // further confirmation. Surface that loudly so the copilot + // relays it to the user instead of staying silent. + if flow.enabled && ops::trigger_is_automatic(&flow.graph) { + let trigger_desc = flow + .graph + .trigger() + .map(tools::describe_trigger) + .unwrap_or_else(|| "automatic".to_string()); + let warning = format!( + "WARNING: this flow is ENABLED with an automatic trigger \ + ({trigger_desc}). It is now LIVE and will fire on its own — tell the \ + user, and offer to disable it (flows_set_enabled) if that's not what \ + they intended." + ); + tracing::warn!( + target: "flows", + %flow_id, + trigger = %trigger_desc, + "[flows] save_workflow: saved onto an enabled auto-trigger flow — now LIVE" + ); + warnings.push(warning); + } Ok(ToolResult::success(serde_json::to_string_pretty(&json!({ "type": "workflow_saved", "flow_id": flow.id, diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 5b93bf7e6..4975d75d1 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -130,6 +130,52 @@ fn trigger_kind_fires(kind: &TriggerKind) -> bool { ) } +/// Whether `graph`'s trigger fires **without a human in the loop** — i.e. on +/// a timer, an inbound webhook, or a connected-app event, as opposed to +/// `manual` (only ever fired by an explicit `flows_run`). Used by +/// [`flows_create`] (issue B29 — save/enable safety, Rule 1) to decide +/// whether a freshly-saved flow may persist `enabled: true` or must persist +/// `enabled: false` until the user arms it explicitly via +/// `flows_set_enabled`. +/// +/// Deliberately broader than [`trigger_kind_fires`]: `webhook` is not yet +/// wired to auto-dispatch in this host (see that fn's doc), but it WILL fire +/// unattended the moment it is — so a webhook-trigger flow must not be handed +/// to the user pre-armed either. Returns `false` for a graph with no single +/// resolvable trigger node or no `trigger_kind` discriminator (never a +/// surprise — it never self-fires). +pub(crate) fn trigger_is_automatic(graph: &WorkflowGraph) -> bool { + let Some(trigger) = graph.trigger() else { + return false; + }; + let Some(kind_value) = trigger.config.get("trigger_kind") else { + return false; + }; + let Ok(kind) = serde_json::from_value::(kind_value.clone()) else { + return false; + }; + matches!( + kind, + TriggerKind::Schedule | TriggerKind::AppEvent | TriggerKind::Webhook + ) +} + +/// Whether `graph` contains a node that can produce a real outbound side +/// effect — `tool_call` (a curated integration action), `http_request`, or +/// `code` (sandboxed but Turing-complete, can reach the network). Used by +/// [`flows_create`] (issue B29, Rule 2) to force `require_approval: true` on +/// any graph that can act on the world, regardless of what the caller +/// passed. A graph built only from `trigger` / `agent` / `transform` / +/// `condition` / data-flow nodes is read-only and unaffected. +pub(crate) fn graph_has_outbound_side_effect(graph: &WorkflowGraph) -> bool { + graph.nodes.iter().any(|n| { + matches!( + n.kind, + NodeKind::ToolCall | NodeKind::HttpRequest | NodeKind::Code + ) + }) +} + /// Produces host-side, **non-fatal** validation warnings for a graph — today /// exactly one: "this trigger kind does not fire automatically yet". Returns /// an empty vec when the trigger fires (`manual`/`schedule`/`app_event`), when @@ -1462,13 +1508,42 @@ pub fn flows_import( /// Creates a new flow from a name and a raw graph JSON value. /// -/// `store::create_flow` defaults new flows to `enabled = true` — this binds -/// the flow's automatic-dispatch side effect (e.g. registers the -/// schedule-trigger cron job) immediately, reusing the same [`bind_trigger`] -/// helper `flows_set_enabled` uses. Without this, a freshly-created enabled -/// schedule flow would silently never fire until an app restart (boot -/// reconcile) or a manual disable→enable toggle. Best-effort, same as -/// `flows_set_enabled`: a binding failure is logged, not fatal to create. +/// Issue B29 (save/enable safety) — two server-side rules apply here, +/// authoritative regardless of what the caller passed, so no creation path +/// (prompt bar, scratch/template modal, proposal "save & enable", copilot +/// `save_workflow`, …) can silently hand the user an armed, unattended +/// automation: +/// +/// - **Rule 1** ([`trigger_is_automatic`]): a graph whose trigger fires +/// without a human in the loop (`schedule` / `app_event` / `webhook`) +/// persists **disabled**. The user arms it explicitly via +/// `flows_set_enabled` — the same toggle already used everywhere else. A +/// `manual` trigger (or no trigger-kind discriminator at all) still +/// persists enabled: it only ever runs via an explicit `flows_run`, so +/// there is no surprise, and gating it would just add friction. +/// +/// This means a caller that represents an explicit user-arming action +/// (e.g. `WorkflowProposalCard`'s "Save & enable" click, +/// `app/src/components/chat/WorkflowProposalCard.tsx`) must check the +/// returned [`Flow`]'s `enabled` field and follow up with +/// `flows_set_enabled(id, true)` when it comes back `false` — otherwise +/// the button's own label lies to the user. That follow-up call is a +/// legitimate, explicit enable, not the silent copilot auto-arm this rule +/// exists to prevent (the copilot's `save_workflow` path has no such +/// follow-up and stays disabled). +/// - **Rule 2** ([`graph_has_outbound_side_effect`]): a graph containing any +/// `tool_call` / `http_request` / `code` node — the three kinds that can +/// produce a real outbound effect — forces `require_approval: true`, +/// overriding whatever the caller passed. A read-only graph (only +/// `trigger` / `agent` / `transform` / `condition` / data-flow nodes) is +/// unaffected. +/// +/// An enabled flow still has its automatic-dispatch side effect bound +/// immediately (e.g. the schedule-trigger cron job registered), reusing the +/// same [`bind_trigger`] helper `flows_set_enabled` uses — but per Rule 1 +/// that now only happens for a `manual`-triggered (or trigger-kind-less) +/// flow. Best-effort, same as `flows_set_enabled`: a binding failure is +/// logged, not fatal to create. pub async fn flows_create( config: &Config, name: String, @@ -1476,16 +1551,62 @@ pub async fn flows_create( require_approval: bool, ) -> Result, String> { let graph = validate_and_migrate_graph(graph_json)?; - tracing::debug!(target: "flows", %name, node_count = graph.nodes.len(), require_approval, "[flows] flows_create: persisting new flow"); - let flow = - store::create_flow(config, name, graph, require_approval).map_err(|e| e.to_string())?; + + // Rule 1: automatic triggers create DISABLED — the user must arm them + // explicitly. + let enabled = !trigger_is_automatic(&graph); + + // Rule 2: any outbound side-effect node forces require_approval, no + // matter what the caller asked for. + let has_side_effect = graph_has_outbound_side_effect(&graph); + let effective_require_approval = require_approval || has_side_effect; + if has_side_effect && !require_approval { + tracing::info!( + target: "flows", + %name, + "[flows] flows_create: forcing require_approval=true — graph contains outbound \ + side-effect node(s) (tool_call / http_request / code)" + ); + } + + tracing::debug!( + target: "flows", + %name, + node_count = graph.nodes.len(), + enabled, + require_approval = effective_require_approval, + "[flows] flows_create: persisting new flow" + ); + let flow = store::create_flow(config, name, graph, effective_require_approval, enabled) + .map_err(|e| e.to_string())?; if flow.enabled { tracing::debug!(target: "flows", flow_id = %flow.id, "[flows] flows_create: flow is enabled — binding automatic-dispatch trigger"); bind_trigger(config, &flow); } - Ok(RpcOutcome::single_log(flow, "flow created")) + let mut logs = vec!["flow created".to_string()]; + if !enabled { + let trigger_label = flow + .graph + .trigger() + .and_then(|t| t.config.get("trigger_kind")) + .and_then(Value::as_str) + .unwrap_or("automatic"); + logs.push(format!( + "Flow created DISABLED because it has an automatic trigger ({trigger_label}). \ + Enable it explicitly (flows_set_enabled) when you are ready for it to fire." + )); + } + if effective_require_approval && !require_approval { + logs.push( + "require_approval forced to true because the graph contains outbound side-effect \ + nodes (tool_call / http_request / code)." + .to_string(), + ); + } + + Ok(RpcOutcome::new(flow, logs)) } /// Duplicates a saved flow: creates an independent copy of its graph under a diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index b3dd1234b..c9eb7b40b 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -362,15 +362,21 @@ async fn flows_run_populates_error_when_a_continue_policy_node_errors() { assert_eq!(reloaded.value.last_status.as_deref(), Some("failed")); } -// ── automatic-dispatch binding (issue B2 finding #1) ───────────────────── +// ── automatic-dispatch binding (issue B2 finding #1, revised by B29) ────── // // Live testing found that `flows_create` persisted a freshly-created, // `enabled = true` schedule flow WITHOUT registering its cron job — only // `flows_set_enabled` bound it. So a brand-new enabled schedule flow would // silently never fire until an app restart (boot reconcile) or a manual -// disable→enable toggle. These tests exercise the fix directly against the -// real `cron` store (not a mock), the same way `bind_schedule_trigger` -// itself does. +// disable→enable toggle. +// +// Issue B29 (save/enable safety) then found the OTHER half of that same bug: +// `flows_create` used to default a schedule flow straight to `enabled: true` +// on create, arming it live before the user ever saw a toggle. Rule 1 now +// creates an automatic-trigger flow DISABLED — so these tests explicitly +// enable via `flows_set_enabled` (the real caller-facing arming path) before +// exercising the cron-binding behavior below, against the real `cron` store +// (not a mock), the same way `bind_schedule_trigger` itself does. fn schedule_trigger_graph(cron_expr: &str) -> Value { json!({ @@ -400,13 +406,27 @@ async fn flows_create_binds_schedule_cron_job_for_an_enabled_flow() { ) .await .unwrap(); - assert!(created.value.enabled, "flows_create defaults to enabled"); + assert!( + !created.value.enabled, + "issue B29: a schedule-trigger flow must create DISABLED, not armed" + ); + assert!( + crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id) + .unwrap() + .is_none(), + "a disabled-on-create schedule flow must not have its cron job bound yet" + ); + + // The user arms it explicitly — this is where the cron job binds. + let enabled = flows_set_enabled(&config, &created.value.id, true) + .await + .unwrap(); + assert!(enabled.value.enabled); let job = crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id).unwrap(); assert!( job.is_some(), - "an enabled schedule flow must have its cron job bound immediately on create, not only \ - after a set_enabled toggle" + "an enabled schedule flow must have its cron job bound immediately on enable" ); assert_eq!(job.unwrap().expression, "0 9 * * *"); } @@ -423,11 +443,14 @@ async fn flows_delete_unbinds_schedule_cron_job() { ) .await .unwrap(); + flows_set_enabled(&config, &created.value.id, true) + .await + .unwrap(); assert!( crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id) .unwrap() .is_some(), - "precondition: cron job bound on create" + "precondition: cron job bound on enable" ); flows_delete(&config, &created.value.id).await.unwrap(); @@ -453,9 +476,12 @@ async fn flows_update_rebinds_schedule_cron_job_when_trigger_schedule_changes() ) .await .unwrap(); + flows_set_enabled(&config, &created.value.id, true) + .await + .unwrap(); let old_job = crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id) .unwrap() - .expect("cron job bound on create"); + .expect("cron job bound on enable"); assert_eq!(old_job.expression, "0 9 * * *"); flows_update( @@ -497,9 +523,12 @@ async fn flows_update_does_not_rebind_when_graph_is_not_supplied() { ) .await .unwrap(); + flows_set_enabled(&config, &created.value.id, true) + .await + .unwrap(); let old_job = crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id) .unwrap() - .expect("cron job bound on create"); + .expect("cron job bound on enable"); // Name-only update: no graph_json supplied, so the trigger cannot have // changed — the existing binding must be left untouched. @@ -1446,7 +1475,9 @@ async fn flows_set_enabled_surfaces_unfired_trigger_warning_at_enable() { .await .unwrap(); - // Re-enable (create already enables) to exercise the enable path's warning. + // A webhook trigger is automatic (B29 Rule 1) so `flows_create` leaves it + // disabled — enable it explicitly here to exercise the enable path's + // warning. let enabled = flows_set_enabled(&config, &created.value.id, true) .await .unwrap(); @@ -3195,3 +3226,356 @@ async fn flows_create_rejects_condition_edges_with_branch_label_on_to_port() { "expected an InvalidConditionRouting-style error, got: {err}" ); } + +// ───────────────────────────────────────────────────────────────────────────── +// Issue B29 — save/enable safety: `flows_create` gating (Rule 1 + Rule 2) +// ───────────────────────────────────────────────────────────────────────────── +// +// Saving a scheduled/automatic flow used to silently arm it live and +// unattended: `store::create_flow` hardcoded `enabled: true`, and +// `require_approval` defaulted to `false` on most creation paths. These +// tests exercise the two server-side rules `flows_create` now enforces, +// regardless of what the caller passed. + +fn app_event_trigger_graph() -> Value { + json!({ + "name": "app-event", + "nodes": [ + { + "id": "t", + "kind": "trigger", + "name": "Trigger", + "config": { "trigger_kind": "app_event", "toolkit": "gmail", "event": "GMAIL_NEW_GMAIL_MESSAGE" } + } + ], + "edges": [] + }) +} + +fn manual_trigger_graph() -> Value { + json!({ + "name": "manual", + "nodes": [ + { + "id": "t", + "kind": "trigger", + "name": "Trigger", + "config": { "trigger_kind": "manual" } + } + ], + "edges": [] + }) +} + +fn tool_call_graph() -> Value { + json!({ + "name": "with-tool-call", + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger" }, + { + "id": "post", + "kind": "tool_call", + "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", "args": { "channel": "general" } } + } + ], + "edges": [ { "from_node": "t", "to_node": "post" } ] + }) +} + +fn http_request_graph() -> Value { + json!({ + "name": "with-http", + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger" }, + { + "id": "call", + "kind": "http_request", + "name": "Call", + "config": { "method": "GET", "url": "https://example.com" } + } + ], + "edges": [ { "from_node": "t", "to_node": "call" } ] + }) +} + +fn code_graph() -> Value { + json!({ + "name": "with-code", + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger" }, + { + "id": "run", + "kind": "code", + "name": "Run", + "config": { "language": "javascript", "source": "return {};" } + } + ], + "edges": [ { "from_node": "t", "to_node": "run" } ] + }) +} + +fn readonly_graph() -> Value { + json!({ + "name": "readonly", + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger" }, + { "id": "a", "kind": "agent", "name": "Summarize", "config": { "prompt": "hi" } }, + { "id": "x", "kind": "transform", "name": "Reshape", "config": { "expression": "=item" } } + ], + "edges": [ + { "from_node": "t", "to_node": "a" }, + { "from_node": "a", "to_node": "x" } + ] + }) +} + +#[tokio::test] +async fn flows_create_schedule_trigger_creates_disabled() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create( + &config, + "scheduled".to_string(), + schedule_trigger_graph("30 7 * * 1-5"), + false, + ) + .await + .unwrap(); + + assert!( + !created.value.enabled, + "a schedule-trigger flow must create disabled" + ); + assert!( + crate::openhuman::cron::find_flow_schedule_job(&config, &created.value.id) + .unwrap() + .is_none(), + "no cron job may be bound for a disabled-on-create schedule flow" + ); + assert!( + created + .logs + .iter() + .any(|l| l.starts_with("Flow created DISABLED")), + "flows_create must loudly log the disabled-on-create decision: {:?}", + created.logs + ); +} + +#[tokio::test] +async fn flows_create_app_event_trigger_creates_disabled() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create( + &config, + "app-event".to_string(), + app_event_trigger_graph(), + false, + ) + .await + .unwrap(); + + assert!( + !created.value.enabled, + "an app_event-trigger flow must create disabled" + ); +} + +#[tokio::test] +async fn flows_create_manual_trigger_creates_enabled() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create(&config, "manual".to_string(), manual_trigger_graph(), false) + .await + .unwrap(); + + assert!( + created.value.enabled, + "a manual-trigger flow only ever fires via explicit flows_run — it must create enabled" + ); +} + +#[tokio::test] +async fn flows_create_no_trigger_kind_creates_enabled() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create(&config, "legacy".to_string(), trigger_only_graph(), false) + .await + .unwrap(); + + assert!( + created.value.enabled, + "a trigger with no trigger_kind discriminator never self-fires — not a surprise, must \ + create enabled" + ); +} + +#[tokio::test] +async fn flows_create_outbound_node_forces_require_approval() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create(&config, "tool-flow".to_string(), tool_call_graph(), false) + .await + .unwrap(); + + assert!( + created.value.require_approval, + "a graph with a tool_call node must force require_approval, even though the caller \ + passed false" + ); + assert!( + created + .logs + .iter() + .any(|l| l.contains("require_approval forced to true")), + "flows_create must loudly log the forced require_approval: {:?}", + created.logs + ); +} + +#[tokio::test] +async fn flows_create_outbound_http_forces_require_approval() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create( + &config, + "http-flow".to_string(), + http_request_graph(), + false, + ) + .await + .unwrap(); + + assert!( + created.value.require_approval, + "a graph with an http_request node must force require_approval" + ); +} + +#[tokio::test] +async fn flows_create_outbound_code_forces_require_approval() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create(&config, "code-flow".to_string(), code_graph(), false) + .await + .unwrap(); + + assert!( + created.value.require_approval, + "a graph with a code node must force require_approval" + ); +} + +#[tokio::test] +async fn flows_create_readonly_graph_respects_caller_require_approval() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let created = flows_create( + &config, + "readonly-flow".to_string(), + readonly_graph(), + false, + ) + .await + .unwrap(); + + assert!( + !created.value.require_approval, + "a read-only graph (no tool_call/http_request/code) must not have require_approval \ + forced — the caller's choice stands" + ); +} + +#[tokio::test] +async fn flows_create_schedule_outbound_creates_disabled_and_approval() { + // The exact bug scenario from the ticket: a scheduled flow that posts to + // Slack, saved with `require_approval: false` — it must come back BOTH + // disabled AND with require_approval forced true. + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let graph = json!({ + "name": "scheduled-slack-post", + "nodes": [ + { + "id": "t", + "kind": "trigger", + "name": "Trigger", + "config": { "trigger_kind": "schedule", "schedule": "30 7 * * 1-5" } + }, + { + "id": "post", + "kind": "tool_call", + "name": "Post", + "config": { "slug": "SLACK_SEND_MESSAGE", "args": { "channel": "general" } } + } + ], + "edges": [ { "from_node": "t", "to_node": "post" } ] + }); + + let created = flows_create(&config, "scheduled-slack".to_string(), graph, false) + .await + .unwrap(); + + assert!( + !created.value.enabled, + "a scheduled flow with an outbound node must still create disabled (Rule 1)" + ); + assert!( + created.value.require_approval, + "a scheduled flow with an outbound node must force require_approval (Rule 2)" + ); +} + +// ── graph_has_outbound_side_effect / trigger_is_automatic helper tests ──── + +#[test] +fn graph_has_outbound_side_effect_detects_tool_call() { + let g = graph(tool_call_graph()); + assert!(graph_has_outbound_side_effect(&g)); +} + +#[test] +fn graph_has_outbound_side_effect_detects_http_request() { + let g = graph(http_request_graph()); + assert!(graph_has_outbound_side_effect(&g)); +} + +#[test] +fn graph_has_outbound_side_effect_detects_code() { + let g = graph(code_graph()); + assert!(graph_has_outbound_side_effect(&g)); +} + +#[test] +fn graph_has_outbound_side_effect_false_for_agent_only() { + let g = graph(readonly_graph()); + assert!(!graph_has_outbound_side_effect(&g)); +} + +#[test] +fn trigger_is_automatic_schedule() { + let g = graph(schedule_trigger_graph("0 9 * * *")); + assert!(trigger_is_automatic(&g)); +} + +#[test] +fn trigger_is_automatic_manual() { + let g = graph(manual_trigger_graph()); + assert!(!trigger_is_automatic(&g)); +} + +#[test] +fn trigger_is_automatic_no_trigger_kind() { + let g = graph(trigger_only_graph()); + assert!(!trigger_is_automatic(&g)); +} diff --git a/src/openhuman/flows/store.rs b/src/openhuman/flows/store.rs index 864975158..c68488101 100644 --- a/src/openhuman/flows/store.rs +++ b/src/openhuman/flows/store.rs @@ -212,17 +212,24 @@ pub fn insert_duplicate_flow(config: &Config, source: &Flow, new_name: String) - /// Creates a brand-new [`Flow`] row from a name + validated graph, stamping /// fresh id/timestamps, and returns the persisted record. +/// +/// `enabled` is decided by the caller ([`crate::openhuman::flows::ops::flows_create`], +/// issue B29 — save/enable safety): a graph with an automatic trigger +/// (`schedule` / `app_event` / `webhook`) is created disabled so it cannot +/// silently arm itself live and unattended; a `manual`-triggered graph is +/// created enabled since it only ever runs on explicit `flows_run`. pub fn create_flow( config: &Config, name: String, graph: tinyflows::model::WorkflowGraph, require_approval: bool, + enabled: bool, ) -> Result { let now = Utc::now().to_rfc3339(); let flow = Flow { id: Uuid::new_v4().to_string(), name, - enabled: true, + enabled, graph, created_at: now.clone(), updated_at: now, diff --git a/src/openhuman/flows/store_tests.rs b/src/openhuman/flows/store_tests.rs index e0edebd97..cd73275ff 100644 --- a/src/openhuman/flows/store_tests.rs +++ b/src/openhuman/flows/store_tests.rs @@ -34,7 +34,7 @@ fn create_get_list_delete_roundtrip() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); assert_eq!(flow.name, "demo"); assert!(flow.enabled); @@ -70,7 +70,7 @@ fn remove_flow_errors_when_not_found() { fn set_enabled_toggles_and_persists() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); assert!(flow.enabled); let disabled = set_enabled(&config, &flow.id, false).unwrap(); @@ -87,7 +87,7 @@ fn set_enabled_toggles_and_persists() { fn update_flow_graph_bumps_updated_at_and_preserves_created_at() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); let mut new_graph = trigger_graph(); new_graph.name = "renamed-graph".to_string(); @@ -103,7 +103,7 @@ fn update_flow_graph_bumps_updated_at_and_preserves_created_at() { fn record_run_sets_last_run_fields() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); assert!(flow.last_run_at.is_none()); record_run(&config, &flow.id, "completed").unwrap(); @@ -176,7 +176,7 @@ fn create_flow_persists_require_approval() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), true).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), true, true).unwrap(); assert!(flow.require_approval); let reloaded = get_flow(&config, &flow.id).unwrap().unwrap(); @@ -187,7 +187,7 @@ fn create_flow_persists_require_approval() { fn update_flow_graph_can_change_require_approval() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); assert!(!flow.require_approval); let updated = @@ -229,9 +229,16 @@ fn list_enabled_flows_excludes_disabled() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let enabled_flow = create_flow(&config, "enabled".to_string(), trigger_graph(), false).unwrap(); - let disabled_flow = - create_flow(&config, "disabled".to_string(), trigger_graph(), false).unwrap(); + let enabled_flow = + create_flow(&config, "enabled".to_string(), trigger_graph(), false, true).unwrap(); + let disabled_flow = create_flow( + &config, + "disabled".to_string(), + trigger_graph(), + false, + true, + ) + .unwrap(); set_enabled(&config, &disabled_flow.id, false).unwrap(); let enabled = list_enabled_flows(&config).unwrap(); @@ -245,7 +252,7 @@ fn list_enabled_flows_excludes_disabled() { fn flow_run_insert_finish_get_round_trip() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); let thread_id = format!("flow:{}:run-1", flow.id); insert_flow_run( @@ -299,7 +306,7 @@ fn flow_run_insert_finish_get_round_trip() { fn finish_flow_run_records_error_on_failure() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); let thread_id = format!("flow:{}:run-2", flow.id); insert_flow_run( &config, @@ -337,8 +344,8 @@ fn get_flow_run_returns_none_for_unknown_id() { fn list_flow_runs_orders_newest_first_and_is_scoped_to_flow() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow_a = create_flow(&config, "a".to_string(), trigger_graph(), false).unwrap(); - let flow_b = create_flow(&config, "b".to_string(), trigger_graph(), false).unwrap(); + let flow_a = create_flow(&config, "a".to_string(), trigger_graph(), false, true).unwrap(); + let flow_b = create_flow(&config, "b".to_string(), trigger_graph(), false, true).unwrap(); insert_flow_run( &config, @@ -385,7 +392,7 @@ fn insert_duplicate_flow_makes_a_disabled_copy_with_new_id_and_same_graph() { // Enabled source with require_approval + a distinctive graph name. let mut graph = trigger_graph(); graph.name = "original-graph".to_string(); - let source = create_flow(&config, "My Flow".to_string(), graph, true).unwrap(); + let source = create_flow(&config, "My Flow".to_string(), graph, true, true).unwrap(); assert!(source.enabled); record_run(&config, &source.id, "completed").unwrap(); let source = get_flow(&config, &source.id).unwrap().unwrap(); @@ -437,7 +444,7 @@ fn seed_run(config: &Config, flow_id: &str, id: &str, day: u32, status: &str) { fn prune_flow_runs_keeps_newest_n_terminal_runs() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); // 5 completed runs on ascending days. for i in 1..=5 { @@ -456,7 +463,7 @@ fn prune_flow_runs_keeps_newest_n_terminal_runs() { fn prune_flow_runs_never_removes_pending_approval_run() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); // An OLD parked pending_approval run (day 1) plus newer completed runs. seed_run(&config, &flow.id, "parked", 1, "pending_approval"); @@ -482,7 +489,7 @@ fn prune_flow_runs_never_removes_pending_approval_run() { fn prune_flow_runs_leaves_running_rows_alone() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); seed_run(&config, &flow.id, "live", 1, "running"); for i in 2..=4 { @@ -499,7 +506,7 @@ fn prune_flow_runs_leaves_running_rows_alone() { fn insert_flow_run_auto_prunes_beyond_retention_cap() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); // Seed exactly MAX_FLOW_RUNS_PER_FLOW completed runs. let cap = MAX_FLOW_RUNS_PER_FLOW; @@ -543,7 +550,7 @@ fn insert_flow_run_auto_prunes_beyond_retention_cap() { fn list_flow_runs_respects_limit() { let tmp = TempDir::new().unwrap(); let config = test_config(&tmp); - let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false).unwrap(); + let flow = create_flow(&config, "demo".to_string(), trigger_graph(), false, true).unwrap(); for i in 0..3 { let id = format!("run-{i}"); diff --git a/src/openhuman/flows/tools.rs b/src/openhuman/flows/tools.rs index 8928a0d3c..8a36cf5e9 100644 --- a/src/openhuman/flows/tools.rs +++ b/src/openhuman/flows/tools.rs @@ -422,7 +422,11 @@ fn node_kind_str(kind: &NodeKind) -> String { /// One-line human description of a trigger node, for the summary's /// `"trigger"` field — e.g. `"schedule: 0 9 * * *"`, `"app event: /// gmail/GMAIL_NEW_GMAIL_MESSAGE"`, `"manual"`. -fn describe_trigger(node: &Node) -> String { +/// +/// `pub(crate)` so [`crate::openhuman::flows::builder_tools::SaveWorkflowTool`] +/// reuses it verbatim for its enabled+auto-trigger arming warning (issue +/// B29) instead of re-deriving the same human string. +pub(crate) fn describe_trigger(node: &Node) -> String { let trigger_kind = node .config .get("trigger_kind")