diff --git a/app/src/components/flows/canvas/nodeConfig/__tests__/nodeConfigForms.test.tsx b/app/src/components/flows/canvas/nodeConfig/__tests__/nodeConfigForms.test.tsx index e16106b25..87052028d 100644 --- a/app/src/components/flows/canvas/nodeConfig/__tests__/nodeConfigForms.test.tsx +++ b/app/src/components/flows/canvas/nodeConfig/__tests__/nodeConfigForms.test.tsx @@ -151,4 +151,27 @@ describe('TriggerForm', () => { 'Every 5 minutes on Wed' ); }); + + it('shows a read-only summary (not the cron builder) for a tagged `{kind:"every"}` schedule', () => { + // Regression: the engine can store `config.schedule` as `{kind:"every", + // every_ms}`, which the cron builder can't edit. It must render a correct + // read-only summary instead of silently resetting the schedule to a + // default cron string via ScheduleField's empty-mount effect. + const { onChange } = renderForm('trigger', { + config: { trigger_kind: 'schedule', schedule: { kind: 'every', every_ms: 86_400_000 } }, + }); + expect(screen.queryByTestId('node-config-trigger-schedule')).not.toBeInTheDocument(); + expect(screen.getByTestId('node-config-trigger-schedule-readonly')).toHaveTextContent('24h'); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('still hands a tagged `{kind:"cron"}` schedule to the editable builder', () => { + renderForm('trigger', { + config: { trigger_kind: 'schedule', schedule: { kind: 'cron', expr: '30 9 * * *' } }, + }); + expect(screen.getByTestId('node-config-trigger-schedule')).toBeInTheDocument(); + expect(screen.getByTestId('node-config-trigger-schedule-summary')).toHaveTextContent( + 'Every day at 09:30' + ); + }); }); diff --git a/app/src/components/flows/canvas/nodeConfig/nodeConfigForms.tsx b/app/src/components/flows/canvas/nodeConfig/nodeConfigForms.tsx index f9c810afb..ad620cb92 100644 --- a/app/src/components/flows/canvas/nodeConfig/nodeConfigForms.tsx +++ b/app/src/components/flows/canvas/nodeConfig/nodeConfigForms.tsx @@ -18,6 +18,7 @@ import createDebug from 'debug'; import { useEffect, useState } from 'react'; +import { describeSchedule, scheduleCronExpr } from '../../../../lib/flows/cron'; import type { NodeKind } from '../../../../lib/flows/types'; import { useT } from '../../../../lib/i18n/I18nContext'; import type { FlowConnection } from '../../../../services/api/flowsApi'; @@ -88,13 +89,34 @@ function TriggerForm({ config, onChange, connections }: NodeConfigFormProps) { label: t(`flows.nodeConfig.trigger.kind_${k}`), }))} /> - {kind === 'schedule' && ( - onChange({ schedule: v })} - testId="node-config-trigger-schedule" - /> - )} + {kind === 'schedule' && + (() => { + const rawSchedule = config.schedule; + const cronExpr = scheduleCronExpr(rawSchedule); + // A cron-shaped schedule (bare string or `{kind:"cron", expr}`) — or + // nothing set yet — is what the visual/advanced builder understands; + // let it render (and start empty, as before, when unset). + if (cronExpr !== null || rawSchedule == null) { + return ( + onChange({ schedule: v })} + testId="node-config-trigger-schedule" + /> + ); + } + // `{kind:"every", every_ms}` / `{kind:"at", at}` — the cron builder + // can't model these. Show a read-only summary instead of handing it + // to `ScheduleField`, whose mount effect would otherwise seed (and + // silently overwrite) a default cron string over the real schedule. + return ( +
+ {describeSchedule(rawSchedule)} +
+ ); + })()} {kind === 'app_event' && ( <> ): CronSpec { return { ...DEFAULT_CRON_SPEC, ...overrides }; @@ -90,3 +99,66 @@ describe('describeCron', () => { expect(describeCron('')).toBe('No schedule set'); }); }); + +describe('describeEveryMs', () => { + it('formats even day/hour/minute intervals', () => { + expect(describeEveryMs(86_400_000)).toContain('24h'); + expect(describeEveryMs(86_400_000)).toContain('Daily'); + expect(describeEveryMs(2 * 86_400_000)).toBe('Every 2 days'); + expect(describeEveryMs(3_600_000)).toBe('Every hour'); + expect(describeEveryMs(4 * 3_600_000)).toBe('Every 4h'); + expect(describeEveryMs(30 * 60_000)).toBe('Every 30m'); + expect(describeEveryMs(60_000)).toBe('Every minute'); + }); + + it('falls back to seconds for sub-minute intervals', () => { + expect(describeEveryMs(15_000)).toBe('Every 15s'); + }); + + it('reports an invalid interval for non-positive values', () => { + expect(describeEveryMs(0)).toBe('Invalid interval'); + expect(describeEveryMs(-5)).toBe('Invalid interval'); + }); +}); + +describe('scheduleCronExpr', () => { + it('passes a bare cron string through unchanged', () => { + expect(scheduleCronExpr('*/5 * * * *')).toBe('*/5 * * * *'); + }); + + it('extracts expr from a tagged cron schedule object', () => { + expect(scheduleCronExpr({ kind: 'cron', expr: '0 9 * * *' })).toBe('0 9 * * *'); + }); + + it('returns null for every/at shapes and unset schedules', () => { + expect(scheduleCronExpr({ kind: 'every', every_ms: 86_400_000 })).toBeNull(); + expect(scheduleCronExpr({ kind: 'at', at: '2026-01-01T00:00:00Z' })).toBeNull(); + expect(scheduleCronExpr(undefined)).toBeNull(); + expect(scheduleCronExpr(null)).toBeNull(); + }); +}); + +describe('describeSchedule', () => { + it('describes a bare cron string the same as describeCron', () => { + expect(describeSchedule('*/5 * * * 3')).toBe('Every 5 minutes on Wed'); + }); + + it('describes a tagged cron schedule object', () => { + expect(describeSchedule({ kind: 'cron', expr: '30 9 * * *' })).toBe('Every day at 09:30'); + }); + + it('describes an "every" schedule — the shape that used to render as unset', () => { + expect(describeSchedule({ kind: 'every', every_ms: 86_400_000 })).toContain('24h'); + }); + + it('describes an "at" schedule', () => { + const result = describeSchedule({ kind: 'at', at: '2026-01-01T09:00:00Z' }); + expect(result).toContain('Once at'); + }); + + it('falls back to "No schedule set" for unset/unrecognized schedules', () => { + expect(describeSchedule(undefined)).toBe('No schedule set'); + expect(describeSchedule(null)).toBe('No schedule set'); + expect(describeSchedule({})).toBe('No schedule set'); + }); +}); diff --git a/app/src/lib/flows/cron.ts b/app/src/lib/flows/cron.ts index a3244e7d6..d32f2bc18 100644 --- a/app/src/lib/flows/cron.ts +++ b/app/src/lib/flows/cron.ts @@ -172,3 +172,88 @@ export function describeCron(expr: string): string { const time = formatTime(spec.hour, spec.minute); return daysPhrase === 'every day' ? `Every day at ${time}` : `At ${time} on ${daysPhrase}`; } + +/** + * The tagged shapes a trigger node's `config.schedule` can hold. The flows + * engine's `crate::openhuman::cron::Schedule` (an internally-tagged enum, + * `#[serde(tag = "kind")]`) is what `flows::tools` and the workflow-builder + * agent actually write today — `{kind:"cron",expr,tz?,active_hours?}` / + * `{kind:"at",at}` / `{kind:"every",every_ms}`. A bare cron string (what the + * visual builder above compiles to, and what the bundled flow templates use) + * is also accepted — the Rust side's custom `Deserialize` treats it as + * shorthand for `Cron{expr}`. + */ +export type ScheduleValue = + | string + | { kind?: string; expr?: string; tz?: string; at?: string; every_ms?: number } + | null + | undefined; + +/** + * Pull the bare cron expression out of a schedule value, if it has one (a + * plain string, or a `{kind:"cron", expr}` object). Returns `null` for the + * `at` / `every` shapes and anything unset — those aren't cron-shaped, so the + * visual/advanced cron builder can't edit them. + */ +export function scheduleCronExpr(value: unknown): string | null { + if (typeof value === 'string') return value; + if (value && typeof value === 'object') { + const expr = (value as Record).expr; + if (typeof expr === 'string') return expr; + } + return null; +} + +const MINUTE_MS = 60_000; +const HOUR_MS = 3_600_000; +const DAY_MS = 86_400_000; + +/** Human phrase for a `{kind:"every", every_ms}` interval — formats the raw + * millisecond count into minutes/hours/days, whichever divides evenly + * ("Every 30m", "Every hour", "Daily (every 24h)"). Falls back to seconds for + * anything finer-grained than a minute. */ +export function describeEveryMs(everyMs: number): string { + if (!Number.isFinite(everyMs) || everyMs <= 0) return 'Invalid interval'; + if (everyMs % DAY_MS === 0) { + const days = everyMs / DAY_MS; + return days === 1 ? 'Daily (every 24h)' : `Every ${days} days`; + } + if (everyMs % HOUR_MS === 0) { + const hours = everyMs / HOUR_MS; + return hours === 1 ? 'Every hour' : `Every ${hours}h`; + } + if (everyMs % MINUTE_MS === 0) { + const minutes = everyMs / MINUTE_MS; + return minutes === 1 ? 'Every minute' : `Every ${minutes}m`; + } + const seconds = Math.round(everyMs / 1000); + return seconds === 1 ? 'Every second' : `Every ${seconds}s`; +} + +/** + * Plain-language summary of a trigger's `schedule` config value, across every + * shape it can actually hold (see {@link ScheduleValue}). This is the single + * place that decides "No schedule set" vs. a real summary — callers should + * never re-derive it from just the cron string, or a valid `every`/`at` + * schedule reads as unset (the canvas trigger-node bug this guards against). + */ +export function describeSchedule(value: unknown): string { + if (typeof value === 'string') return describeCron(value); + if (value && typeof value === 'object') { + const obj = value as Record; + const kind = typeof obj.kind === 'string' ? obj.kind : undefined; + + if (kind === 'every' && typeof obj.every_ms === 'number') { + return describeEveryMs(obj.every_ms); + } + if (kind === 'at' && typeof obj.at === 'string') { + const date = new Date(obj.at); + return Number.isNaN(date.getTime()) + ? `Once at ${obj.at}` + : `Once at ${date.toLocaleString()}`; + } + // `{kind:"cron", expr}` (or an untagged object that merely carries `expr`). + if (typeof obj.expr === 'string') return describeCron(obj.expr); + } + return describeCron(''); // 'No schedule set' +} diff --git a/app/src/lib/flows/nodeSummary.test.ts b/app/src/lib/flows/nodeSummary.test.ts index 693f50e41..f277d5f2d 100644 --- a/app/src/lib/flows/nodeSummary.test.ts +++ b/app/src/lib/flows/nodeSummary.test.ts @@ -15,6 +15,22 @@ describe('describeNode', () => { expect(describeNode('trigger', { trigger_kind: 'manual' })).toBe('Runs on demand'); }); + it('describes a schedule trigger stored as a tagged `{kind:"every"}` schedule', () => { + // Regression: the engine writes `config.schedule` as a tagged object + // (`{kind:"every", every_ms}`), not a bare cron string — the summary must + // not fall through to "No schedule set" for that real, configured shape. + const summary = describeNode('trigger', { + trigger_kind: 'schedule', + schedule: { kind: 'every', every_ms: 86_400_000 }, + }); + expect(summary).not.toBe('No schedule set'); + expect(summary).toContain('24h'); + }); + + it('still shows "No schedule set" for a genuinely unconfigured schedule trigger', () => { + expect(describeNode('trigger', { trigger_kind: 'schedule' })).toBe('No schedule set'); + }); + it('describes an http_request from method + url', () => { expect(describeNode('http_request', { method: 'POST', url: 'https://api.x.com/v1' })).toBe( 'POST https://api.x.com/v1' diff --git a/app/src/lib/flows/nodeSummary.ts b/app/src/lib/flows/nodeSummary.ts index 5a84f8bd4..d1905acfe 100644 --- a/app/src/lib/flows/nodeSummary.ts +++ b/app/src/lib/flows/nodeSummary.ts @@ -5,10 +5,10 @@ * without opening the config drawer. Falls back to a generic per-kind label * when the config isn't filled in yet. * - * Pure + dependency-light (only {@link describeCron}) so it's trivially + * Pure + dependency-light (only {@link describeSchedule}) so it's trivially * testable and can be called on every render of `FlowNodeComponent`. */ -import { describeCron } from './cron'; +import { describeSchedule } from './cron'; import type { NodeKind } from './types'; function str(config: Record, key: string): string { @@ -34,7 +34,7 @@ export function describeNode( case 'trigger': { const tk = str(config, 'trigger_kind') || 'manual'; if (tk === 'manual') return 'Runs on demand'; - if (tk === 'schedule') return describeCron(str(config, 'schedule')); + if (tk === 'schedule') return describeSchedule(config.schedule); if (tk === 'webhook') return 'Runs on an incoming webhook'; if (tk === 'app_event') { const parts = [str(config, 'toolkit'), str(config, 'trigger_slug')].filter(Boolean);