fix(flows): render + preserve non-cron trigger schedules on the canvas (B9) (#4647)

This commit is contained in:
Cyrus Gray
2026-07-07 19:52:53 +05:30
committed by GitHub
parent 0663750f04
commit 77639d3bc5
6 changed files with 229 additions and 11 deletions
@@ -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'
);
});
});
@@ -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' && (
<ScheduleField
value={configString(config, 'schedule')}
onChange={v => 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 (
<ScheduleField
value={cronExpr ?? ''}
onChange={v => 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 (
<div
className="rounded-lg border border-primary-200 bg-primary-50/60 px-2.5 py-1.5 text-xs font-medium text-primary-700 dark:border-primary-500/30 dark:bg-primary-500/10 dark:text-primary-300"
data-testid="node-config-trigger-schedule-readonly">
{describeSchedule(rawSchedule)}
</div>
);
})()}
{kind === 'app_event' && (
<>
<ComposioToolkitField
+73 -1
View File
@@ -6,7 +6,16 @@
*/
import { describe, expect, it } from 'vitest';
import { buildCron, type CronSpec, DEFAULT_CRON_SPEC, describeCron, parseCron } from './cron';
import {
buildCron,
type CronSpec,
DEFAULT_CRON_SPEC,
describeCron,
describeEveryMs,
describeSchedule,
parseCron,
scheduleCronExpr,
} from './cron';
function spec(overrides: Partial<CronSpec>): 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');
});
});
+85
View File
@@ -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<string, unknown>).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<string, unknown>;
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'
}
+16
View File
@@ -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'
+3 -3
View File
@@ -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<string, unknown>, 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);