diff --git a/app/src/components/flows/FlowRunInspectorDrawer.tsx b/app/src/components/flows/FlowRunInspectorDrawer.tsx index 43bb06552..1ad99710d 100644 --- a/app/src/components/flows/FlowRunInspectorDrawer.tsx +++ b/app/src/components/flows/FlowRunInspectorDrawer.tsx @@ -57,26 +57,38 @@ export const FLOW_RUN_STATUS_ACCENT: Record = { 'border-ocean-200 bg-ocean-50 text-ocean-700 dark:border-ocean-500/30 dark:bg-ocean-500/10 dark:text-ocean-300', completed: 'border-sage-200 bg-sage-50 text-sage-700 dark:border-sage-500/30 dark:bg-sage-500/10 dark:text-sage-300', + // Settled like `completed`, but at least one step had a `=`-binding that + // resolved to `null` (run honesty, PR2) — reuse `pending_approval`'s amber + // so "needs a look" reads consistently across statuses. + completed_with_warnings: + 'border-amber-200 bg-amber-50 text-amber-700 dark:border-amber-500/30 dark:bg-amber-500/10 dark:text-amber-300', pending_approval: 'border-amber-200 bg-amber-50 text-amber-700 dark:border-amber-500/30 dark:bg-amber-500/10 dark:text-amber-300', failed: 'border-coral-200 bg-coral-50 text-coral-700 dark:border-coral-500/30 dark:bg-coral-500/10 dark:text-coral-300', + // Neutral treatment, matching `WorkflowRunDetail.tsx`'s `RUN_STATUS_ACCENT.cancelled`. + cancelled: 'border-line bg-surface-muted text-content-secondary', }; /** Header status dot per run status — mirrors `PHASE_STATUS_DOT`. Exported, see above. */ export const FLOW_RUN_STATUS_DOT: Record = { running: 'bg-ocean-500 animate-pulse', completed: 'bg-sage-500', + // Settled (no pulse) — the amber signals "worth a look", not "in progress". + completed_with_warnings: 'bg-amber-500', pending_approval: 'bg-amber-500 animate-pulse', failed: 'bg-coral-500', + cancelled: 'bg-surface-strong', }; /** i18n key per run status. Exported, see above. */ export const FLOW_RUN_STATUS_KEY: Record = { running: 'flowRuns.status.running', completed: 'flowRuns.status.completed', + completed_with_warnings: 'flowRuns.status.completed_with_warnings', pending_approval: 'flowRuns.status.pending_approval', failed: 'flowRuns.status.failed', + cancelled: 'flowRuns.status.cancelled', }; function formatTimestamp(value: string | null | undefined): string | null { diff --git a/app/src/components/flows/FlowRunsDrawer.test.tsx b/app/src/components/flows/FlowRunsDrawer.test.tsx index 511a99889..ad61f6ee9 100644 --- a/app/src/components/flows/FlowRunsDrawer.test.tsx +++ b/app/src/components/flows/FlowRunsDrawer.test.tsx @@ -28,20 +28,26 @@ vi.mock('./FlowRunInspectorDrawer', () => ({ FLOW_RUN_STATUS_ACCENT: { running: 'accent-running', completed: 'accent-completed', + completed_with_warnings: 'accent-completed-with-warnings', pending_approval: 'accent-pending', failed: 'accent-failed', + cancelled: 'accent-cancelled', }, FLOW_RUN_STATUS_DOT: { running: 'dot-running', completed: 'dot-completed', + completed_with_warnings: 'dot-completed-with-warnings', pending_approval: 'dot-pending', failed: 'dot-failed', + cancelled: 'dot-cancelled', }, FLOW_RUN_STATUS_KEY: { running: 'flowRuns.status.running', completed: 'flowRuns.status.completed', + completed_with_warnings: 'flowRuns.status.completed_with_warnings', pending_approval: 'flowRuns.status.pending_approval', failed: 'flowRuns.status.failed', + cancelled: 'flowRuns.status.cancelled', }, FlowRunInspectorDrawer: (props: { runId: string | null; onClose: () => void }) => { FlowRunInspectorDrawer(props); @@ -109,6 +115,14 @@ describe('FlowRunsDrawer', () => { expect(screen.getByText('Runs for Daily digest')).toBeInTheDocument(); }); + it('renders the amber pill for a run completed with warnings', async () => { + listFlowRuns.mockResolvedValue([makeRun({ id: 'run-1', status: 'completed_with_warnings' })]); + renderDrawer('flow-1', vi.fn()); + + const row = await screen.findByTestId('flow-run-row-run-1'); + expect(row).toHaveTextContent('Completed with warnings'); + }); + it('falls back to a generic title when no flowName is given', async () => { listFlowRuns.mockResolvedValue([]); renderDrawer('flow-1', vi.fn()); diff --git a/app/src/hooks/__tests__/useFlowRunPoller.test.ts b/app/src/hooks/__tests__/useFlowRunPoller.test.ts index 00fb98c4c..d1fa33dff 100644 --- a/app/src/hooks/__tests__/useFlowRunPoller.test.ts +++ b/app/src/hooks/__tests__/useFlowRunPoller.test.ts @@ -108,6 +108,42 @@ describe('useFlowRunPoller', () => { expect(getFlowRun).toHaveBeenCalledTimes(1); }); + it('stops polling once the run completes with warnings', async () => { + getFlowRun.mockResolvedValue( + makeRun({ status: 'completed_with_warnings', finished_at: '2026-01-01T00:01:00Z' }) + ); + const { result } = renderHook(() => useFlowRunPoller('thread-1')); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.run?.status).toBe('completed_with_warnings'); + expect(getFlowRun).toHaveBeenCalledTimes(1); + + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(getFlowRun).toHaveBeenCalledTimes(1); + }); + + it('stops polling once the run is cancelled', async () => { + getFlowRun.mockResolvedValue( + makeRun({ status: 'cancelled', finished_at: '2026-01-01T00:01:00Z' }) + ); + const { result } = renderHook(() => useFlowRunPoller('thread-1')); + + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.run?.status).toBe('cancelled'); + expect(getFlowRun).toHaveBeenCalledTimes(1); + + await act(async () => { + await vi.advanceTimersByTimeAsync(10_000); + }); + expect(getFlowRun).toHaveBeenCalledTimes(1); + }); + it('stops polling once the run fails', async () => { getFlowRun.mockResolvedValue(makeRun({ status: 'failed', error: 'boom' })); const { result } = renderHook(() => useFlowRunPoller('thread-1')); diff --git a/app/src/hooks/useFlowRunPoller.ts b/app/src/hooks/useFlowRunPoller.ts index 1b4f6fb8c..085476f27 100644 --- a/app/src/hooks/useFlowRunPoller.ts +++ b/app/src/hooks/useFlowRunPoller.ts @@ -13,6 +13,8 @@ * * `pending_approval` is explicitly NOT terminal — a paused run still needs * live status so the drawer reflects an approval elsewhere resolving it. + * `completed_with_warnings` (run honesty, PR2) and `cancelled` are terminal, + * same as `completed`/`failed`. */ import debug from 'debug'; import { useEffect, useRef, useState } from 'react'; @@ -24,7 +26,12 @@ const log = debug('flows:poller'); /** How often to poll a non-terminal run for progress. */ const POLL_INTERVAL_MS = 2000; -const TERMINAL = new Set(['completed', 'failed']); +const TERMINAL = new Set([ + 'completed', + 'completed_with_warnings', + 'failed', + 'cancelled', +]); function isTerminal(run: FlowRun | null): boolean { return run !== null && TERMINAL.has(run.status); diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index f4df8f1b2..f8e252053 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -3612,6 +3612,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'عنصر الإدخال المصدر', 'flowRuns.status.running': 'قيد التشغيل', 'flowRuns.status.completed': 'مكتمل', + 'flowRuns.status.completed_with_warnings': 'مكتمل مع تحذيرات', 'flowRuns.status.pending_approval': 'بانتظار الموافقة', 'flowRuns.status.failed': 'فشل', 'flowRuns.status.cancelled': 'ملغى', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 6529ed035..4744ab72c 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -3695,6 +3695,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'উৎস ইনপুট আইটেম', 'flowRuns.status.running': 'চলছে', 'flowRuns.status.completed': 'সম্পন্ন', + 'flowRuns.status.completed_with_warnings': 'সতর্কতা সহ সম্পন্ন', 'flowRuns.status.pending_approval': 'অনুমোদনের অপেক্ষায়', 'flowRuns.status.failed': 'ব্যর্থ', 'flowRuns.status.cancelled': 'বাতিল করা হয়েছে', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 8086d7872..eb87d5185 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -3785,6 +3785,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Quell-Eingabeelement', 'flowRuns.status.running': 'Läuft', 'flowRuns.status.completed': 'Abgeschlossen', + 'flowRuns.status.completed_with_warnings': 'Abgeschlossen mit Warnungen', 'flowRuns.status.pending_approval': 'Wartet auf Genehmigung', 'flowRuns.status.failed': 'Fehlgeschlagen', 'flowRuns.status.cancelled': 'Abgebrochen', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index e5f753db4..cd45db770 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -4341,6 +4341,7 @@ const en: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Source input item', 'flowRuns.status.running': 'Running', 'flowRuns.status.completed': 'Completed', + 'flowRuns.status.completed_with_warnings': 'Completed with warnings', 'flowRuns.status.pending_approval': 'Awaiting approval', 'flowRuns.status.failed': 'Failed', 'flowRuns.status.cancelled': 'Cancelled', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index 72456f24b..62e9fabf7 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -3758,6 +3758,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Elemento de entrada de origen', 'flowRuns.status.running': 'En ejecución', 'flowRuns.status.completed': 'Completado', + 'flowRuns.status.completed_with_warnings': 'Completado con advertencias', 'flowRuns.status.pending_approval': 'Esperando aprobación', 'flowRuns.status.failed': 'Fallido', 'flowRuns.status.cancelled': 'Cancelado', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index d1406b07d..67c7e152f 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -3773,6 +3773,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': "Élément d'entrée source", 'flowRuns.status.running': 'En cours', 'flowRuns.status.completed': 'Terminé', + 'flowRuns.status.completed_with_warnings': 'Terminé avec avertissements', 'flowRuns.status.pending_approval': "En attente d'approbation", 'flowRuns.status.failed': 'Échoué', 'flowRuns.status.cancelled': 'Annulé', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index b98fb1ae9..8e09d6025 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -3696,6 +3696,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'स्रोत इनपुट आइटम', 'flowRuns.status.running': 'चल रहा है', 'flowRuns.status.completed': 'पूर्ण', + 'flowRuns.status.completed_with_warnings': 'चेतावनियों के साथ पूर्ण', 'flowRuns.status.pending_approval': 'अनुमोदन की प्रतीक्षा में', 'flowRuns.status.failed': 'विफल', 'flowRuns.status.cancelled': 'रद्द किया गया', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 622dffcf2..96e98e454 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -3704,6 +3704,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Item masukan sumber', 'flowRuns.status.running': 'Berjalan', 'flowRuns.status.completed': 'Selesai', + 'flowRuns.status.completed_with_warnings': 'Selesai dengan peringatan', 'flowRuns.status.pending_approval': 'Menunggu persetujuan', 'flowRuns.status.failed': 'Gagal', 'flowRuns.status.cancelled': 'Dibatalkan', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index c436e94a6..db91ff968 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -3753,6 +3753,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Elemento di input di origine', 'flowRuns.status.running': 'In esecuzione', 'flowRuns.status.completed': 'Completato', + 'flowRuns.status.completed_with_warnings': 'Completato con avvertenze', 'flowRuns.status.pending_approval': 'In attesa di approvazione', 'flowRuns.status.failed': 'Non riuscito', 'flowRuns.status.cancelled': 'Annullato', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index 8f004c741..06b331d25 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -3659,6 +3659,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': '소스 입력 항목', 'flowRuns.status.running': '실행 중', 'flowRuns.status.completed': '완료됨', + 'flowRuns.status.completed_with_warnings': '경고와 함께 완료됨', 'flowRuns.status.pending_approval': '승인 대기 중', 'flowRuns.status.failed': '실패', 'flowRuns.status.cancelled': '취소됨', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index 504664527..683cd7a46 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -3739,6 +3739,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Źródłowy element wejściowy', 'flowRuns.status.running': 'W trakcie', 'flowRuns.status.completed': 'Zakończono', + 'flowRuns.status.completed_with_warnings': 'Ukończono z ostrzeżeniami', 'flowRuns.status.pending_approval': 'Oczekuje na zatwierdzenie', 'flowRuns.status.failed': 'Niepowodzenie', 'flowRuns.status.cancelled': 'Anulowano', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 70955a167..0d46a3e22 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -3754,6 +3754,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Item de entrada de origem', 'flowRuns.status.running': 'Em execução', 'flowRuns.status.completed': 'Concluído', + 'flowRuns.status.completed_with_warnings': 'Concluído com avisos', 'flowRuns.status.pending_approval': 'Aguardando aprovação', 'flowRuns.status.failed': 'Falhou', 'flowRuns.status.cancelled': 'Cancelado', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 6c0899701..fffd872bb 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -3728,6 +3728,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': 'Исходный входной элемент', 'flowRuns.status.running': 'Выполняется', 'flowRuns.status.completed': 'Завершено', + 'flowRuns.status.completed_with_warnings': 'Завершено с предупреждениями', 'flowRuns.status.pending_approval': 'Ожидает подтверждения', 'flowRuns.status.failed': 'Не удалось', 'flowRuns.status.cancelled': 'Отменено', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index d9fbe1036..963844ba5 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -3502,6 +3502,7 @@ const messages: TranslationMap = { 'flowRuns.inspector.sourceInputTitle': '来源输入项', 'flowRuns.status.running': '运行中', 'flowRuns.status.completed': '已完成', + 'flowRuns.status.completed_with_warnings': '完成但有警告', 'flowRuns.status.pending_approval': '等待批准', 'flowRuns.status.failed': '失败', 'flowRuns.status.cancelled': '已取消', diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index 11fb61898..8b5ecc81f 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -49,7 +49,13 @@ const FLOW_RESUME_TIMEOUT_MS = 610_000; // --------------------------------------------------------------------------- /** Lifecycle status of a durable flow run. */ -export type FlowRunStatus = 'running' | 'completed' | 'pending_approval' | 'failed'; +export type FlowRunStatus = + | 'running' + | 'completed' + | 'completed_with_warnings' + | 'pending_approval' + | 'failed' + | 'cancelled'; /** One reconstructed step of a persisted `FlowRun` (`src/openhuman/flows/types.rs::FlowRunStep`). */ export interface FlowRunStep { diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index e59baef22..489d8b84e 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -1067,19 +1067,16 @@ pub async fn flows_run( }; let outcome = journaled.outcome; - let status = if outcome.pending_approvals.is_empty() { - "completed" - } else { - "pending_approval" - }; + let settled = settle_steps(config, &thread_id, &outcome.output); + let (status, error) = finalize_terminal_status(&settled, &outcome.pending_approvals); store::record_run(config, flow_id, status).map_err(|e| e.to_string())?; finish_flow_run_row( config, &thread_id, status, - &settle_steps(config, &thread_id, &outcome.output), + &settled, &outcome.pending_approvals, - None, + error.as_deref(), ); export_run_to_langfuse( config, @@ -1269,19 +1266,16 @@ pub async fn flows_resume( }; let outcome = journaled.outcome; - let status = if outcome.pending_approvals.is_empty() { - "completed" - } else { - "pending_approval" - }; + let settled = settle_steps(config, thread_id, &outcome.output); + let (status, error) = finalize_terminal_status(&settled, &outcome.pending_approvals); store::record_run(config, flow_id, status).map_err(|e| e.to_string())?; finish_flow_run_row( config, thread_id, status, - &settle_steps(config, thread_id, &outcome.output), + &settled, &outcome.pending_approvals, - None, + error.as_deref(), ); export_run_to_langfuse( config, @@ -1412,14 +1406,19 @@ pub async fn sweep_expired_parked_runs(config: &Config) -> usize { /// a `running` row whose task is gone): no live task exists to unwind, so /// this settles the row terminally itself and drops the checkpoint. /// -/// A run that is already terminal (`completed` / `failed` / `cancelled`) is a -/// clear error, not a silent no-op. +/// A run that is already terminal (`completed` / `completed_with_warnings` / +/// `failed` / `cancelled`) is a clear error, not a silent no-op — otherwise a +/// settled warning run could be overwritten as `"cancelled"`, corrupting the +/// run-honesty status it already recorded. pub async fn flows_cancel_run(config: &Config, run_id: &str) -> Result, String> { let run = store::get_flow_run(config, run_id) .map_err(|e| e.to_string())? .ok_or_else(|| format!("flow run '{run_id}' not found"))?; - if matches!(run.status.as_str(), "completed" | "failed" | "cancelled") { + if matches!( + run.status.as_str(), + "completed" | "completed_with_warnings" | "failed" | "cancelled" + ) { return Err(format!( "flow run '{run_id}' is already terminal (status: {}) — nothing to cancel", run.status @@ -1612,6 +1611,70 @@ fn settle_steps(config: &Config, run_id: &str, output: &Value) -> Vec &'static str { + if steps.iter().any(|s| s.status.as_deref() == Some("error")) { + return "failed"; + } + if steps.iter().any(|s| !s.diagnostics.is_empty()) { + "completed_with_warnings" + } else { + "completed" + } +} + +/// Names the node(s) whose step settled with `status == "error"` — the +/// engine's `ExecutionStep` carries no error message of its own for a step +/// that failed under an `on_error: "continue"`/`"route"` policy (it only +/// fails the *run* future, and so gets an actual error string, when the +/// policy is `"stop"`), so this is the best available detail for +/// [`FlowRun::error`] when [`degrade_completed_status`] degrades to +/// `"failed"` without an outer run-future `Err`. +fn failed_step_error_summary(steps: &[FlowRunStep]) -> Option { + let failed_nodes: Vec<&str> = steps + .iter() + .filter(|s| s.status.as_deref() == Some("error")) + .map(|s| s.node_id.as_str()) + .collect(); + if failed_nodes.is_empty() { + None + } else { + Some(format!( + "node(s) failed after retries: {}", + failed_nodes.join(", ") + )) + } +} + +/// Computes a settled run's terminal status and, when that status is +/// `"failed"`, an accompanying error message — shared by `flows_run` and +/// `flows_resume` so the two call sites can't drift on the +/// `pending_approval` > `degrade_completed_status` precedence or forget to +/// populate [`FlowRun::error`] (its doc contract: "Error message when +/// `status == \"failed\"`") for a run that degraded via a settled step error +/// rather than an outer run-future `Err`. +fn finalize_terminal_status( + settled: &[FlowRunStep], + pending_approvals: &[String], +) -> (&'static str, Option) { + if !pending_approvals.is_empty() { + return ("pending_approval", None); + } + let status = degrade_completed_status(settled); + let error = if status == "failed" { + failed_step_error_summary(settled) + } else { + None + }; + (status, error) +} + /// Milliseconds since the Unix epoch, for `CoreNotificationEvent::timestamp_ms`. fn now_ms() -> u64 { std::time::SystemTime::now() diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index 9f45b0ad1..2d54dd122 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -319,6 +319,49 @@ async fn flows_run_records_failed_status_when_a_node_errors() { ); } +#[tokio::test] +async fn flows_run_populates_error_when_a_continue_policy_node_errors() { + // Unlike the default `on_error: stop` (previous test), `"continue"` turns + // the node failure into data on the default port instead of failing the + // run future — the run settles `Ok`, but the errored step still degrades + // the terminal status to `"failed"` via `degrade_completed_status`. That + // path must still populate `FlowRun.error` (its doc contract: "Error + // message when status == \"failed\"") even though the engine's + // `ExecutionStep` carries no message of its own for this case. + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + let graph = json!({ + "name": "boom-continue", + "nodes": [ + { "id": "t", "kind": "trigger", "name": "Trigger" }, + { "id": "x", "kind": "tool_call", "name": "X", "config": { "on_error": "continue" } } + ], + "edges": [ { "from_node": "t", "to_node": "x" } ] + }); + + let created = flows_create(&config, "boom-continue".to_string(), graph, false) + .await + .unwrap(); + + let run = flows_run(&config, &created.value.id, json!({}), FlowRunTrigger::Rpc) + .await + .expect("on_error:continue must settle the run future Ok, not bubble up an Err"); + let thread_id = run.value["thread_id"].as_str().unwrap().to_string(); + + let run_row = flows_get_run(&config, &thread_id).await.unwrap(); + assert_eq!(run_row.value.status, "failed"); + let error = run_row + .value + .error + .as_deref() + .expect("a degraded-to-failed run must populate FlowRun.error, not leave it None"); + assert!(error.contains('x'), "got: {error}"); + + let reloaded = flows_get(&config, &created.value.id).await.unwrap(); + assert_eq!(reloaded.value.last_status.as_deref(), Some("failed")); +} + // ── automatic-dispatch binding (issue B2 finding #1) ───────────────────── // // Live testing found that `flows_create` persisted a freshly-created, @@ -1211,6 +1254,47 @@ async fn flows_cancel_run_of_an_already_completed_run_errors() { assert!(err.contains("already terminal"), "got: {err}"); } +#[tokio::test] +async fn flows_cancel_run_of_a_completed_with_warnings_run_errors() { + // A settled `completed_with_warnings` run (run honesty, PR2) must be just + // as terminal as a plain `completed` run — otherwise `flows_cancel_run` + // falls through to its not-in-flight path and overwrites the row (and the + // flow summary) as `"cancelled"`, silently discarding the warning status + // the run already recorded. + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let created = flows_create(&config, "demo".to_string(), trigger_only_graph(), false) + .await + .unwrap(); + + let run = flows_run(&config, &created.value.id, json!({}), FlowRunTrigger::Rpc) + .await + .unwrap(); + let thread_id = run.value["thread_id"].as_str().unwrap().to_string(); + + // Force the settled row to the warning status directly — an end-to-end + // null-binding graph isn't needed to exercise this guard. + store::finish_flow_run( + &config, + &thread_id, + "completed_with_warnings", + &chrono::Utc::now().to_rfc3339(), + &[], + &[], + None, + ) + .unwrap(); + + let err = flows_cancel_run(&config, &thread_id) + .await + .expect_err("cancelling a completed_with_warnings run must be a clear error"); + assert!(err.contains("already terminal"), "got: {err}"); + + // And the row must still read back as the warning status, not overwritten. + let run_row = flows_get_run(&config, &thread_id).await.unwrap(); + assert_eq!(run_row.value.status, "completed_with_warnings"); +} + #[tokio::test] async fn flows_cancel_run_missing_run_errors() { let tmp = TempDir::new().unwrap(); @@ -1654,3 +1738,112 @@ fn flow_stream_target_generates_request_id_when_absent_or_blank() { // Two mints are distinct uuids. assert_ne!(a.request_id, b.request_id); } + +// ───────────────────────────────────────────────────────────────────────────── +// degrade_completed_status (PR2 — run honesty) +// ───────────────────────────────────────────────────────────────────────────── + +fn clean_step(node_id: &str) -> FlowRunStep { + FlowRunStep { + node_id: node_id.to_string(), + output: Value::Null, + port: None, + status: Some("success".to_string()), + duration_ms: Some(1), + diagnostics: Vec::new(), + } +} + +#[test] +fn degrade_completed_status_all_clean_stays_completed() { + let steps = vec![clean_step("a"), clean_step("b")]; + assert_eq!(degrade_completed_status(&steps), "completed"); +} + +#[test] +fn degrade_completed_status_null_binding_becomes_warnings() { + let mut warned = clean_step("a"); + warned.diagnostics = vec![json!({ "location": "args.to", "expression": "=item.to" })]; + let steps = vec![clean_step("trigger"), warned]; + assert_eq!(degrade_completed_status(&steps), "completed_with_warnings"); +} + +#[test] +fn degrade_completed_status_errored_step_becomes_failed() { + let mut errored = clean_step("a"); + errored.status = Some("error".to_string()); + let steps = vec![clean_step("trigger"), errored]; + assert_eq!(degrade_completed_status(&steps), "failed"); +} + +#[test] +fn degrade_completed_status_error_outranks_diagnostics() { + // A step can carry both an error status and null-resolution diagnostics + // (e.g. it errored trying to use the unresolved value) — failed wins. + let mut errored_with_diagnostics = clean_step("a"); + errored_with_diagnostics.status = Some("error".to_string()); + errored_with_diagnostics.diagnostics = + vec![json!({ "location": "args.to", "expression": "=item.to" })]; + let steps = vec![errored_with_diagnostics]; + assert_eq!(degrade_completed_status(&steps), "failed"); +} + +#[test] +fn failed_step_error_summary_none_when_no_step_errored() { + let steps = vec![clean_step("a"), clean_step("b")]; + assert_eq!(failed_step_error_summary(&steps), None); +} + +#[test] +fn failed_step_error_summary_names_the_errored_node() { + let mut errored = clean_step("x"); + errored.status = Some("error".to_string()); + let steps = vec![clean_step("trigger"), errored]; + let summary = failed_step_error_summary(&steps).expect("an errored step must summarize"); + assert!(summary.contains('x'), "got: {summary}"); +} + +#[test] +fn failed_step_error_summary_names_every_errored_node() { + let mut errored_a = clean_step("a"); + errored_a.status = Some("error".to_string()); + let mut errored_b = clean_step("b"); + errored_b.status = Some("error".to_string()); + let steps = vec![errored_a, errored_b]; + let summary = failed_step_error_summary(&steps).unwrap(); + assert!( + summary.contains('a') && summary.contains('b'), + "got: {summary}" + ); +} + +#[test] +fn finalize_terminal_status_pending_approval_wins_over_error() { + // Precedence: an outstanding pending_approval always wins, even if a step + // also settled with an error — mirrors degrade_completed_status's own + // precedence rule, now centralized in finalize_terminal_status. + let mut errored = clean_step("a"); + errored.status = Some("error".to_string()); + let steps = vec![errored]; + let (status, error) = finalize_terminal_status(&steps, &["gate".to_string()]); + assert_eq!(status, "pending_approval"); + assert_eq!(error, None); +} + +#[test] +fn finalize_terminal_status_populates_error_on_degraded_failure() { + let mut errored = clean_step("x"); + errored.status = Some("error".to_string()); + let steps = vec![errored]; + let (status, error) = finalize_terminal_status(&steps, &[]); + assert_eq!(status, "failed"); + assert!(error.unwrap().contains('x')); +} + +#[test] +fn finalize_terminal_status_no_error_when_clean() { + let steps = vec![clean_step("a")]; + let (status, error) = finalize_terminal_status(&steps, &[]); + assert_eq!(status, "completed"); + assert_eq!(error, None); +} diff --git a/src/openhuman/flows/types.rs b/src/openhuman/flows/types.rs index 8e2a7c694..1afc4e836 100644 --- a/src/openhuman/flows/types.rs +++ b/src/openhuman/flows/types.rs @@ -199,10 +199,15 @@ pub struct FlowRun { pub thread_id: String, /// Run status. Not an enum (kept a free-form `String` for forward-compat /// with statuses added by newer builds), but the vocabulary is fixed: - /// `"running"` | `"completed"` | `"pending_approval"` | `"failed"` | - /// `"cancelled"` (issue G4 — a run cancelled via `flows_cancel_run`, or a - /// parked `pending_approval` run swept by the TTL expiry). All of - /// `completed` / `failed` / `cancelled` are terminal. + /// `"running"` | `"completed"` | `"completed_with_warnings"` | + /// `"pending_approval"` | `"failed"` | `"cancelled"` (issue G4 — a run + /// cancelled via `flows_cancel_run`, or a parked `pending_approval` run + /// swept by the TTL expiry). `"completed_with_warnings"` (run honesty, + /// PR2) is a terminal status like `"completed"`, but at least one settled + /// [`FlowRunStep`] carries non-empty `diagnostics` (a `=`-binding that + /// resolved to `null`) even though no step outright errored. All of + /// `completed` / `completed_with_warnings` / `failed` / `cancelled` are + /// terminal. pub status: String, /// RFC3339 timestamp when the run started. pub started_at: String,