From f650bb6e0195e9a87d68dac80256a2b1a4516000 Mon Sep 17 00:00:00 2001 From: abccodes Date: Thu, 18 Jun 2026 04:10:32 -0700 Subject: [PATCH] [aidan] feat/workflows: require at least one step to save a workflow --- backend/apps/workflows/workflows.py | 20 ++++++++++--- .../src/app/pages/AgentChat/AgentChat.tsx | 8 ++++-- .../src/app/pages/Workflows/EditAgentView.tsx | 16 ++++++++--- .../pages/Workflows/ScheduleThisPopover.tsx | 28 ++++++++++++++++++- .../src/app/pages/Workflows/WorkflowCard.tsx | 4 ++- .../pages/Workflows/WorkflowCardSubviews.tsx | 24 +++++++++------- 6 files changed, 78 insertions(+), 22 deletions(-) diff --git a/backend/apps/workflows/workflows.py b/backend/apps/workflows/workflows.py index d4210a5a..ea50ee26 100644 --- a/backend/apps/workflows/workflows.py +++ b/backend/apps/workflows/workflows.py @@ -151,6 +151,10 @@ def _normalize_schedule_state(wf: Workflow) -> None: wf.next_run_at = scheduler.compute_next_fire(wf) if wf.schedule.enabled else None +def _has_nonempty_steps(steps: list[WorkflowStep] | None) -> bool: + return any(bool((s.text or "").strip()) for s in (steps or [])) + + def _parse_calendar_bound(value: str, label: str) -> datetime: raw = (value or "").strip() if raw.endswith("Z"): @@ -166,6 +170,8 @@ def _parse_calendar_bound(value: str, label: str) -> datetime: @workflows.router.post("/create") async def create_workflow(body: WorkflowCreate): + if not body.unsaved and not _has_nonempty_steps(body.steps): + raise HTTPException(status_code=400, detail="Workflow must have at least one step") actions = body.actions # Scheduled workflows default to freeze=on for safety. The user can # flip "Full agent access" in the editor with an explicit confirm. @@ -785,15 +791,21 @@ async def commit_draft(workflow_id: str): wf = storage.get_workflow(workflow_id) if not wf: raise HTTPException(status_code=404, detail="Workflow not found") - # Clicking Save is the user committing to this workflow, so reveal it in - # the hub (clears the "+ New" build-in-progress flag) even if there's no - # pending draft to flush. - wf.unsaved = False if wf.draft_steps is None: + if not _has_nonempty_steps(wf.steps): + raise HTTPException(status_code=400, detail="Workflow must have at least one step") + # Clicking Save is the user committing to this workflow, so reveal it + # in the hub (clears the "+ New" build-in-progress flag). + wf.unsaved = False await p_end_edit_session(wf) storage.save_workflow(wf) return _enriched(wf) before = wf.model_dump(mode="json") + if not _has_nonempty_steps(wf.draft_steps): + raise HTTPException(status_code=400, detail="Workflow must have at least one step") + # Clicking Save is the user committing to this workflow, so reveal it in + # the hub (clears the "+ New" build-in-progress flag). + wf.unsaved = False wf.steps = wf.draft_steps wf.draft_steps = None await p_relabel_changed_steps(wf, before.get("steps") or []) diff --git a/frontend/src/app/pages/AgentChat/AgentChat.tsx b/frontend/src/app/pages/AgentChat/AgentChat.tsx index b885babf..0cff5c47 100644 --- a/frontend/src/app/pages/AgentChat/AgentChat.tsx +++ b/frontend/src/app/pages/AgentChat/AgentChat.tsx @@ -937,9 +937,13 @@ const AgentChat: React.FC = ({ sessionId: sessionIdProp, onClose if (id) dispatch(removeCard(id)); }, [linkedWorkflowId, id, dispatch]); - const onTestSaveWorkflow = useCallback(() => { + const onTestSaveWorkflow = useCallback(async () => { if (linkedWorkflowId) { - dispatch(commitDraft(linkedWorkflowId)); + try { + await dispatch(commitDraft(linkedWorkflowId)).unwrap(); + } catch { + return; + } dispatch(updateWorkflowCard({ workflowId: linkedWorkflowId, patch: { view: 'saved' } })); dispatch(setCardSidecar({ workflowId: linkedWorkflowId, sessionId: null, kind: null })); } diff --git a/frontend/src/app/pages/Workflows/EditAgentView.tsx b/frontend/src/app/pages/Workflows/EditAgentView.tsx index da5cf65f..7ce31a71 100644 --- a/frontend/src/app/pages/Workflows/EditAgentView.tsx +++ b/frontend/src/app/pages/Workflows/EditAgentView.tsx @@ -112,6 +112,7 @@ export default function EditAgentView({ workflow, steps, isFixMode = false, onEd const [saveAnchorEl, setSaveAnchorEl] = useState(null); const [testSessionId, setTestSessionId] = useState(null); const draftSteps = workflow.draft_steps ?? steps; + const canSave = draftSteps.some((s) => (s.text || '').trim().length > 0); // A draft always exists in edit mode (we snapshot on entry), so only flag // "unsaved" once the draft actually diverges from the committed steps. const hasChanges = workflow.draft_steps != null && JSON.stringify(workflow.draft_steps) !== JSON.stringify(workflow.steps); @@ -140,10 +141,15 @@ export default function EditAgentView({ workflow, steps, isFixMode = false, onEd }, []); const onSaveNow = useCallback(async () => { + if (!canSave) return; setSavePhase('idle'); - await dispatch(commitDraft(workflow.id)); + try { + await dispatch(commitDraft(workflow.id)).unwrap(); + } catch { + return; + } toSaved(); - }, [dispatch, workflow.id, toSaved]); + }, [canSave, dispatch, workflow.id, toSaved]); const onRunTest = useCallback(async () => { try { @@ -206,11 +212,13 @@ export default function EditAgentView({ workflow, steps, isFixMode = false, onEd Discard Save diff --git a/frontend/src/app/pages/Workflows/ScheduleThisPopover.tsx b/frontend/src/app/pages/Workflows/ScheduleThisPopover.tsx index d02fedde..81edead4 100644 --- a/frontend/src/app/pages/Workflows/ScheduleThisPopover.tsx +++ b/frontend/src/app/pages/Workflows/ScheduleThisPopover.tsx @@ -30,6 +30,30 @@ const PRESETS: Preset[] = [ { label: 'Every month on the 1st', hint: 'Monthly summary, billing report', build: () => ({ enabled: true, repeat_unit: 'month', repeat_every: 1, hour: 9, minute: 0 }) }, ]; +function extractStepsFromSession(session: { messages?: Array<{ role: string; content: unknown; hidden?: boolean }> } | null | undefined): Array<{ id: string; text: string }> { + const out: Array<{ id: string; text: string }> = []; + for (const msg of session?.messages || []) { + if (msg.role !== 'user' || msg.hidden) continue; + const text = typeof msg.content === 'string' + ? msg.content + : Array.isArray(msg.content) + ? msg.content.map((b: any) => (typeof b === 'string' ? b : b?.text || '')).join(' ') + : ''; + const trimmed = text.trim(); + if (trimmed.length < 6) continue; + out.push({ id: `step-${out.length + 1}-${Date.now().toString(36)}`, text: trimmed.slice(0, 400) }); + if (out.length === 3) break; + } + if (out.length === 0 && session?.messages?.length) { + const fallback = session.messages.find((m) => m.role === 'user'); + if (fallback) { + const text = typeof fallback.content === 'string' ? fallback.content : ''; + out.push({ id: `step-1-${Date.now().toString(36)}`, text: text.slice(0, 400) || 'Run the original task' }); + } + } + return out; +} + interface Props { anchorEl: HTMLElement | null; onClose: () => void; @@ -61,6 +85,7 @@ export default function ScheduleThisPopover({ anchorEl, onClose, sessionId, sess const sessionDashboardId = useAppSelector( (s) => sessionId ? s.agents.sessions[sessionId]?.dashboard_id : null, ); + const sourceSession = useAppSelector((s) => sessionId ? s.agents.sessions[sessionId] : null); // Dup-detect: a chat session can only sanely have one schedule attached. // If we find one already, offer "Open existing" instead of silently @@ -82,6 +107,7 @@ export default function ScheduleThisPopover({ anchorEl, onClose, sessionId, sess const result = await dispatch(createWorkflow({ title, source_session_id: sessionId, + steps: extractStepsFromSession(sourceSession), schedule, } as Partial)); if (createWorkflow.fulfilled.match(result)) { @@ -98,7 +124,7 @@ export default function ScheduleThisPopover({ anchorEl, onClose, sessionId, sess } finally { setBusy(false); } - }, [busy, dispatch, sessionId, title, onClose, onCreated]); + }, [busy, dispatch, sessionId, sourceSession, title, onClose, onCreated]); const openCustom = useCallback(() => { // Open a local draft. NO backend create yet — the workflow only diff --git a/frontend/src/app/pages/Workflows/WorkflowCard.tsx b/frontend/src/app/pages/Workflows/WorkflowCard.tsx index dfbd029c..2b780cea 100644 --- a/frontend/src/app/pages/Workflows/WorkflowCard.tsx +++ b/frontend/src/app/pages/Workflows/WorkflowCard.tsx @@ -381,12 +381,14 @@ const WorkflowCard: React.FC = ({ const persistDraft = useCallback(async (): Promise => { const d = card?.draft; if (!d || persistingRef.current) return null; + const draftSteps = d.steps || []; + if (!draftSteps.some((s) => (s.text || '').trim().length > 0)) return null; persistingRef.current = true; try { const result = await dispatch(createWorkflow({ title: (d.title as string) || 'New workflow', description: (d.description as string) || '', - steps: (d.steps || []).map((s) => ({ id: s.id, text: s.text })), + steps: draftSteps.map((s) => ({ id: s.id, text: s.text })), source_session_id: (d.source_session_id as string | undefined) || card?.sourceSessionId || null, use_synced_prompt: true, model: defaultModel || (d.model as string), diff --git a/frontend/src/app/pages/Workflows/WorkflowCardSubviews.tsx b/frontend/src/app/pages/Workflows/WorkflowCardSubviews.tsx index 1e65c877..04201390 100644 --- a/frontend/src/app/pages/Workflows/WorkflowCardSubviews.tsx +++ b/frontend/src/app/pages/Workflows/WorkflowCardSubviews.tsx @@ -123,6 +123,7 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, const liveDraft = (card?.draft ?? initialDraft ?? {}) as Partial; const title = (liveDraft.title as string) || 'New workflow'; const description = (liveDraft.description as string) || ''; + const canSave = steps.some((s) => (s.text || '').trim().length > 0); // The new workflow runs with the user's configured default model/mode (their // subscription, etc.), falling back to whatever the source chat used. Without // this the backend picks its own default, which surprised users who'd set a @@ -161,6 +162,7 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, }, [closeRequestNonce]); const saveWorkflow = useCallback(async (): Promise => { + if (!canSave) return null; const result = await dispatch(createWorkflow({ title, description, @@ -175,7 +177,7 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, const wf = (result as unknown as { payload: Workflow }).payload; if (wf?.id) return wf; return null; - }, [dispatch, title, description, steps, sourceSessionId, liveDraft, defaultModel, defaultMode]); + }, [canSave, dispatch, title, description, steps, sourceSessionId, liveDraft, defaultModel, defaultMode]); const onIgnore = useCallback(async () => { if (busy) return; @@ -183,7 +185,7 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, }, [busy]); const onSaveThenSchedule = useCallback(async () => { - if (busy) return; + if (busy || !canSave) return; setBusy(true); try { const wf = await saveWorkflow(); @@ -191,10 +193,10 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, } finally { setBusy(false); } - }, [busy, saveWorkflow, onSaved]); + }, [busy, canSave, saveWorkflow, onSaved]); const onSaveDraft = useCallback(async () => { - if (busy) return; + if (busy || !canSave) return; setBusy(true); try { const wf = await saveWorkflow(); @@ -203,7 +205,7 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, setBusy(false); setSavePromptOpen(false); } - }, [busy, saveWorkflow, onSaved]); + }, [busy, canSave, saveWorkflow, onSaved]); const onDontSave = useCallback(() => { setSavePromptOpen(false); @@ -252,15 +254,16 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, Not now Schedule Workflow @@ -288,8 +291,9 @@ export function PreviewView({ workflowId, steps, sourceSessionId, initialDraft, + onClick={canSave ? onSaveDraft : undefined} + title={canSave ? undefined : 'Add at least one step before saving'} + sx={{ fontSize: '0.84rem', fontWeight: 700, color: '#fff', bgcolor: c.accent.primary, borderRadius: 999, cursor: busy ? 'wait' : canSave ? 'pointer' : 'not-allowed', px: 1.5, py: 0.6, opacity: busy || !canSave ? 0.6 : 1, '&:hover': { filter: 'brightness(1.06)' } }}> Save