From c0c58d6b017ec914debe5fd9800a0d59183c6056 Mon Sep 17 00:00:00 2001 From: Aron Prins Date: Mon, 11 May 2026 09:53:10 +0200 Subject: [PATCH] fix(ui): prevent lossy cron rewrites + redesign routine triggers tab (#3569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Humans configure when those agents run via **routines**, which are driven by cron-backed triggers > - The routine detail page exposed triggers through an always-visible inline add form and per-row inline editor, with a ScheduleEditor that only understood a narrow set of cron shapes > - That combination was actively lossy: pasting `0 9,13,17 * * *` silently collapsed to `0 10 * * *` on save, and common shapes (every-N-minutes within a window, multiple times per day, monthly on several dates) had no first-class UI > - This pull request rebuilds the triggers tab around a list of cards + add/edit modal, teaches ScheduleEditor the cron shapes users actually want, and prevents cron round-trips from dropping data > - It also *optionally* tucks the Triggers/Runs/Activity tabs into the shared right-hand PropertiesPanel (same pattern as Issues and Goals) so they stay in view alongside the routine instead of being hidden below the main content > - The benefit is that routine scheduling becomes non-destructive and legible — operators can see, describe, and edit real-world schedules without dropping into raw cron and without fear that saving will silently rewrite their trigger ## What Changed **Core fixes + redesign (required):** - **ScheduleEditor correctness** — `parseCronToPreset` now detects comma lists, ranges, steps, and unknown tokens across every cron field and routes anything it can't round-trip losslessly to the `custom` preset (except `dow === "1-5"` → `weekdays`). Fixes the `0 9,13,17 * * *` → `0 10 * * *` regression. - **ScheduleEditor presets** — adds first-class support for every-N-minutes (with optional hour window + weekdays-only), every-N-hours, hourly at minute offset, daily with multiple times/day, selected-days-of-week with multiple times, and monthly on multiple dates. `describeSchedule` unfolds multi-value hour/day lists into readable sentences. - **ScheduleEditor polish** — swaps raw `` for the shadcn `Checkbox` primitive so hour-window and weekdays-only toggles match the rest of the app. - **Triggers tab redesign** — replaces the inline add form + inline editor with a header + \"Add trigger\" button, compact `TriggerListCard` entries, and a `TriggerDialog` add/edit modal. Enable/disable is now a single-click switch on each card; delete goes through a `ConfirmDialog`. - **Webhook trigger gating** — webhook kind is visible but disabled with \"— COMING SOON\" in the add dialog, matching the old inline form's production behaviour. Editing existing webhook triggers still works. - **Tests** — adds `ScheduleEditor.test.ts` covering the regression cron strings (`0 9,13,17 * * *`, `0 */4 * * *`, `0 10,16 * * *`) plus existing preset patterns as regression guards in the other direction. **Optional layout change (commit `145a86b5` — can be dropped without affecting the rest):** - Moves Triggers/Runs/Activity into the shared right-hand `PropertiesPanel` (persisted open/close, header toggle button), mirroring `IssueDetail` and `GoalDetail`. The reasoning: these tabs are the primary way a human *operates* a routine, and keeping them docked on the right means they're always in view next to the routine content rather than hidden below the fold. Mobile parity is preserved by rendering the same tabs inline below `md`. Trigger cards and run/activity rows were restructured into vertical stacks so they fit the 320px panel without overflow, and the last-result badge became a wrapping inline chip so long error strings no longer fill the card width. - **If reviewers prefer to keep the tabs inline below the routine, this commit can be reverted cleanly without touching any of the fixes above.** ## Screenshots: Old: triggers-old New: Screenshot 2026-04-13 at 12 25 00 New Add Trigger modal: Screenshot 2026-04-13 at 12 25 07 Commit 145a86b5 Properties panel: commit-145a86b51265e326160cb8c48e0874cb36d86f37 ## Verification - `cd ui && npm test -- ScheduleEditor` — new cron parser/describer cases pass. - Full UI test suite + typecheck green locally. - Manual: 1. Open a routine → Triggers tab → verify cards render with enable switch, edit, and delete (confirm dialog). 2. Create a schedule trigger with each preset (every-N-min with window, every-N-hours, hourly@offset, daily multi-time, weekly multi-time, monthly multi-date) → save → reopen → preset + values round-trip intact. 3. Paste `0 9,13,17 * * *` into an existing trigger → editor routes to Custom with the raw cron preserved → save → value unchanged. 4. Try to add a webhook trigger → kind option shows \"— COMING SOON\" and is disabled; edit an existing webhook trigger still works. 5. Toggle the properties panel via header button → state persists across reload. Resize below `md` → tabs render inline. - **Before/after screenshots:** attached in PR description (inline triggers tab → list+modal; raw-cron save hazard → custom preset preservation; bottom-of-page tabs → right-hand PropertiesPanel). ## Risks - **Medium-low.** UI-only change; no API, schema, or migration impact. - `parseCronToPreset` / `describeSchedule` signatures are preserved, but their *behaviour* shifts: more cron strings now resolve to `custom` than before. Any external caller relying on the old (lossy) classification would see different preset tags — none known in-repo. - PropertiesPanel reuse (optional commit) depends on the existing localStorage key behaviour; if two routes ever write conflicting open/close state under the same key, one could clobber the other. Mirrors the established `IssueDetail`/`GoalDetail` pattern, so risk is bounded. Reverting `145a86b5` removes this risk entirely while keeping the fixes. - Webhook kind is disabled in the add dialog only; existing webhook triggers remain editable, so no data is stranded. ## Model Used - **Authoring / PR drafting:** Anthropic Claude — `claude-opus-4-6` (1M context window), via Claude Code CLI. Used for diff review and PR description drafting. Code authored by @aronprins. - **Post-hoc audit:** OpenAI Codex — `gpt-5.4` (high reasoning). Audited the completed work after implementation; found no issues. ## Checklist - [x] Thinking path traces from project context to this change - [x] Model used specified with version + capability details - [x] Tests run locally and pass - [x] Added/updated tests (`ScheduleEditor.test.ts`) - [x] Before/after screenshots attached - [ ] Documentation updated — none required (internal UI only) - [x] Risks documented - [x] Will address all Greptile + reviewer comments before merge --- ui/src/components/ConfirmDialog.tsx | 57 + ui/src/components/ScheduleEditor.test.ts | 166 +++ ui/src/components/ScheduleEditor.tsx | 1204 +++++++++++++++++----- ui/src/components/TriggerDialog.tsx | 268 +++++ ui/src/components/TriggerListCard.tsx | 139 +++ ui/src/pages/RoutineDetail.tsx | 707 ++++++------- 6 files changed, 1906 insertions(+), 635 deletions(-) create mode 100644 ui/src/components/ConfirmDialog.tsx create mode 100644 ui/src/components/ScheduleEditor.test.ts create mode 100644 ui/src/components/TriggerDialog.tsx create mode 100644 ui/src/components/TriggerListCard.tsx diff --git a/ui/src/components/ConfirmDialog.tsx b/ui/src/components/ConfirmDialog.tsx new file mode 100644 index 00000000..3a8ed9a5 --- /dev/null +++ b/ui/src/components/ConfirmDialog.tsx @@ -0,0 +1,57 @@ +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; + +interface ConfirmDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + title: string; + description?: string; + confirmLabel?: string; + cancelLabel?: string; + destructive?: boolean; + onConfirm: () => void; + busy?: boolean; +} + +export function ConfirmDialog({ + open, + onOpenChange, + title, + description, + confirmLabel = "Confirm", + cancelLabel = "Cancel", + destructive, + onConfirm, + busy, +}: ConfirmDialogProps) { + return ( + + + + {title} + {description && {description}} + + + + + + + + ); +} diff --git a/ui/src/components/ScheduleEditor.test.ts b/ui/src/components/ScheduleEditor.test.ts new file mode 100644 index 00000000..e0f8853a --- /dev/null +++ b/ui/src/components/ScheduleEditor.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "vitest"; +import { + describeSchedule, + getScheduleEditorPresetForTest, + hasSingleMinuteAcrossTimesForTest, + parseCronToPreset, + roundTripCronForTest, +} from "./ScheduleEditor"; + +describe("parseCronToPreset", () => { + describe("simple single-value crons map to presets", () => { + it("maps `* * * * *` to every_minute", () => { + expect(parseCronToPreset("* * * * *").preset).toBe("every_minute"); + }); + + it("maps `0 * * * *` to every_hour", () => { + const parsed = parseCronToPreset("0 * * * *"); + expect(parsed.preset).toBe("every_hour"); + expect(parsed.minute).toBe("0"); + }); + + it("maps `0 9 * * *` to every_day at 09:00", () => { + const parsed = parseCronToPreset("0 9 * * *"); + expect(parsed.preset).toBe("every_day"); + expect(parsed.hour).toBe("9"); + expect(parsed.minute).toBe("0"); + }); + + it("maps `0 9 * * 1-5` to weekdays", () => { + const parsed = parseCronToPreset("0 9 * * 1-5"); + expect(parsed.preset).toBe("weekdays"); + expect(parsed.hour).toBe("9"); + }); + + it("maps `0 9 * * 1` to weekly on Monday", () => { + const parsed = parseCronToPreset("0 9 * * 1"); + expect(parsed.preset).toBe("weekly"); + expect(parsed.dayOfWeek).toBe("1"); + expect(parsed.hour).toBe("9"); + }); + + it("maps `0 9 1 * *` to monthly on the 1st", () => { + const parsed = parseCronToPreset("0 9 1 * *"); + expect(parsed.preset).toBe("monthly"); + expect(parsed.dayOfMonth).toBe("1"); + expect(parsed.hour).toBe("9"); + }); + }); + + describe("complex crons round-trip via custom preset (regression: comma lists were silently coerced into every_day)", () => { + it("routes comma-separated hours to custom", () => { + // Regression: `0 9,13,17 * * *` used to be parsed as `every_day` with + // hour `"9,13,17"`, which the hour { + const next = times.slice(); + const value = e.target.value || "00:00"; + next[i] = value; + if (next.length > 1) { + const parsed = parseTimeParts(value); + if (parsed) { + for (let idx = 0; idx < next.length; idx += 1) { + const current = parseTimeParts(next[idx] ?? ""); + const hour = idx === i ? parsed.hour : (current?.hour ?? 0); + next[idx] = `${pad(hour)}:${pad(parsed.minute)}`; + } + } + } + onChange(next); + }} + /> + + + ))} + + + ); +} + +function DayOfWeekPicker({ + days, + onChange, +}: { + days: number[]; + onChange: (next: number[]) => void; +}) { + const letters = ["S", "M", "T", "W", "T", "F", "S"]; + return ( +
+ {letters.map((l, i) => { + const active = days.includes(i); + return ( + + ); + })} +
+ ); +} + +function DayOfMonthPicker({ + domDays, + onChange, +}: { + domDays: number[]; + onChange: (next: number[]) => void; +}) { + return ( +
+ {Array.from({ length: 31 }, (_, i) => i + 1).map((d) => { + const active = domDays.includes(d); + return ( + + ); + })} +
+ ); +} + +// --------------------------------------------------------------------------- +// ScheduleEditor component (rich) +// --------------------------------------------------------------------------- export function ScheduleEditor({ value, @@ -153,68 +643,247 @@ export function ScheduleEditor({ value: string; onChange: (cron: string) => void; }) { - const parsed = useMemo(() => parseCronToPreset(value), [value]); - const [preset, setPreset] = useState(parsed.preset); - const [hour, setHour] = useState(parsed.hour); - const [minute, setMinute] = useState(parsed.minute); - const [dayOfWeek, setDayOfWeek] = useState(parsed.dayOfWeek); - const [dayOfMonth, setDayOfMonth] = useState(parsed.dayOfMonth); - const [customCron, setCustomCron] = useState(preset === "custom" ? value : ""); + const [state, setState] = useState(() => parseCronToEditorState(value)); - // Sync from external value changes + // Sync when external value changes and isn't the same cron we just emitted. useEffect(() => { - const p = parseCronToPreset(value); - setPreset(p.preset); - setHour(p.hour); - setMinute(p.minute); - setDayOfWeek(p.dayOfWeek); - setDayOfMonth(p.dayOfMonth); - if (p.preset === "custom") setCustomCron(value); + const currentCron = buildCronFromState(state); + if (currentCron !== value) { + setState(parseCronToEditorState(value)); + } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [value]); - const emitChange = useCallback( - (p: SchedulePreset, h: string, m: string, dow: string, dom: string, custom: string) => { - if (p === "custom") { - onChange(custom); - } else { - onChange(buildCron(p, h, m, dow, dom)); - } + const emitState = useCallback( + (next: EditorState) => { + setState(next); + onChange(buildCronFromState(next)); }, [onChange], ); - const handlePresetChange = (newPreset: SchedulePreset) => { - setPreset(newPreset); - if (newPreset === "custom") { - setCustomCron(value); - } else { - emitChange(newPreset, hour, minute, dayOfWeek, dayOfMonth, customCron); - } - }; + const update = useCallback( + (patch: Pick | Partial) => { + emitState({ ...state, ...patch }); + }, + [emitState, state], + ); + + const { preset } = state; return ( -
- +
+ {/* Preset */} +
+ + +
- {preset === "custom" ? ( + {preset === "every_minute" && ( +

+ No options — runs every minute, around the clock. +

+ )} + + {preset === "every_n_minutes" && ( +
+
+ +
+ update({ n: clamp(+e.target.value || 1, 1, 59) })} + /> + minutes +
+
+ {[1, 5, 10, 15, 20, 30].map((v) => ( + update({ n: v })} + > + {v} + + ))} +
+
+ +
+ )} + + {preset === "hourly" && (
+ { - setCustomCron(e.target.value); - emitChange("custom", hour, minute, dayOfWeek, dayOfMonth, e.target.value); - }} + type="number" + min={0} + max={59} + className="w-24 font-mono" + value={state.minutePast} + onChange={(e) => update({ minutePast: clamp(+e.target.value || 0, 0, 59) })} + /> +

+ Runs once an hour at :{pad(state.minutePast)} +

+
+ )} + + {preset === "every_n_hours" && ( +
+
+ +
+ update({ n: clamp(+e.target.value || 1, 1, 23) })} + /> + hours +
+
+ {[1, 2, 3, 4, 6, 8, 12].map((v) => ( + update({ n: v })} + > + {v} + + ))} +
+
+
+ + update({ minutePast: clamp(+e.target.value || 0, 0, 59) })} + /> +
+ +
+ )} + + {preset === "daily" && ( +
+ + update({ times })} /> + {state.times.length > 1 && ( +

+ All times in one schedule share the same minute. Changing one minute updates them all. +

+ )} +
+ )} + + {preset === "weekdays" && ( +
+
+ + update({ days })} /> +
+ {( + [ + ["Weekdays", [1, 2, 3, 4, 5]], + ["Weekends", [0, 6]], + ["All days", [0, 1, 2, 3, 4, 5, 6]], + ["Mon · Wed · Fri", [1, 3, 5]], + ["Tue · Thu", [2, 4]], + ] as const + ).map(([label, days]) => ( + update({ days: [...days] })} + > + {label} + + ))} +
+
+
+ + update({ times })} /> + {state.times.length > 1 && ( +

+ All times in one schedule share the same minute. Changing one minute updates them all. +

+ )} +
+
+ )} + + {preset === "monthly" && ( +
+
+ + update({ domDays })} /> +
+ {( + [ + ["1st only", [1]], + ["15th only", [15]], + ["1st & 15th", [1, 15]], + ["Last day (28th)", [28]], + ] as const + ).map(([label, days]) => ( + update({ domDays: [...days] })} + > + {label} + + ))} +
+

+ Days 29–31 are skipped in months that don't have them. +

+
+
+ + update({ times })} /> + {state.times.length > 1 && ( +

+ All times in one schedule share the same minute. Changing one minute updates them all. +

+ )} +
+
+ )} + + {preset === "custom" && ( +
+ + update({ custom: e.target.value })} placeholder="0 10 * * *" className="font-mono text-sm" /> @@ -222,123 +891,150 @@ export function ScheduleEditor({ Five fields: minute hour day-of-month month day-of-week

- ) : ( -
- {preset !== "every_minute" && preset !== "every_hour" && ( - <> - at - - : - - - )} - - {preset === "every_hour" && ( - <> - at minute - - - )} - - {preset === "weekly" && ( - <> - on -
- {DAYS_OF_WEEK.map((d) => ( - - ))} -
- - )} - - {preset === "monthly" && ( - <> - on day - - - )} -
)} + + + +
+
+ Summary — + {describeSchedule(buildCronFromState(state))} +
+ + {buildCronFromState(state)} + +
); } + +function WindowAndWeekdaysToggles({ + state, + update, +}: { + state: EditorState; + update: (patch: Partial) => void; +}) { + return ( + <> +
+ + {state.windowEnabled && ( +
+ + to + +
+ )} +
+ + + ); +} + +function WeekdaysOnlyToggle({ + state, + update, +}: { + state: EditorState; + update: (patch: Partial) => void; +}) { + return ( + + ); +} + +function changePreset(state: EditorState, next: EditorPreset): EditorState { + // Reset ambiguous sub-state when switching presets so we don't carry + // over a stale weekdaysOnly / windowEnabled from a sibling preset. + switch (next) { + case "every_minute": + return { ...state, preset: next }; + case "every_n_minutes": + return { ...state, preset: next, n: 15, windowEnabled: false, weekdaysOnly: false }; + case "hourly": + return { ...state, preset: next }; + case "every_n_hours": + return { ...state, preset: next, n: 2, weekdaysOnly: false }; + case "daily": + return { ...state, preset: next, times: state.times.length ? state.times : ["09:00"] }; + case "weekdays": + return { + ...state, + preset: next, + days: state.days.length ? state.days : [1, 2, 3, 4, 5], + times: state.times.length ? state.times : ["09:00"], + }; + case "monthly": + return { + ...state, + preset: next, + domDays: state.domDays.length ? state.domDays : [1], + times: state.times.length ? state.times : ["09:00"], + }; + case "custom": + return { ...state, preset: next, custom: state.custom || buildCronFromState(state) }; + } +} + +function clamp(v: number, lo: number, hi: number): number { + return Math.min(Math.max(v, lo), hi); +} + +export function getScheduleEditorPresetForTest(cron: string): EditorPreset { + return parseCronToEditorState(cron).preset; +} + +export function hasSingleMinuteAcrossTimesForTest(times: string[]): boolean { + return hasSingleMinuteAcrossTimes(times); +} + +export function roundTripCronForTest(cron: string): string { + return buildCronFromState(parseCronToEditorState(cron)); +} diff --git a/ui/src/components/TriggerDialog.tsx b/ui/src/components/TriggerDialog.tsx new file mode 100644 index 00000000..e9490a5b --- /dev/null +++ b/ui/src/components/TriggerDialog.tsx @@ -0,0 +1,268 @@ +import { useEffect, useState } from "react"; +import type { RoutineTrigger } from "@paperclipai/shared"; +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { ToggleSwitch } from "@/components/ui/toggle-switch"; +import { ScheduleEditor } from "./ScheduleEditor"; + +const triggerKinds = ["schedule", "webhook"] as const; +const signingModes = ["bearer", "hmac_sha256", "github_hmac", "none"] as const; +const SIGNING_MODES_WITHOUT_REPLAY_WINDOW = new Set(["github_hmac", "none"]); +const signingModeDescriptions: Record = { + bearer: "Expect a shared bearer token in the Authorization header.", + hmac_sha256: "Expect an HMAC SHA-256 signature over the request using the shared secret.", + github_hmac: "Accept GitHub-style X-Hub-Signature-256 header (HMAC over raw body, no timestamp).", + none: "No authentication — the webhook URL itself acts as a shared secret.", +}; + +type TriggerKind = (typeof triggerKinds)[number]; + +export interface TriggerDialogState { + label: string; + kind: TriggerKind; + cronExpression: string; + signingMode: string; + replayWindowSec: string; + enabled: boolean; +} + +interface TriggerDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + /** When editing an existing trigger, pass it here. Null for create. */ + trigger: RoutineTrigger | null; + /** Timezone to use when creating a new schedule trigger (the detail page uses the browser's zone). */ + fallbackTimezone: string; + /** Called when the user submits. For updates `id` is non-null. */ + onSubmit: (payload: { + id: string | null; + kind: TriggerKind; + // For create: full body. For update: partial patch ready to send. + body: Record; + }) => void; + submitting?: boolean; +} + +const BLANK: TriggerDialogState = { + label: "", + kind: "schedule", + cronExpression: "0 9 * * 1-5", + signingMode: "bearer", + replayWindowSec: "300", + enabled: true, +}; + +function draftFromTrigger(trigger: RoutineTrigger | null): TriggerDialogState { + if (!trigger) return { ...BLANK }; + return { + label: trigger.label ?? "", + kind: (trigger.kind as TriggerKind) ?? "schedule", + cronExpression: trigger.cronExpression ?? "0 9 * * 1-5", + signingMode: trigger.signingMode ?? "bearer", + replayWindowSec: String(trigger.replayWindowSec ?? 300), + enabled: trigger.enabled, + }; +} + +function parseReplayWindowSec(raw: string): number { + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed < 1) return 300; + return Math.trunc(parsed); +} + +export function TriggerDialog({ + open, + onOpenChange, + trigger, + fallbackTimezone, + onSubmit, + submitting, +}: TriggerDialogProps) { + const isEdit = !!trigger; + const [draft, setDraft] = useState(() => draftFromTrigger(trigger)); + + // Reset the draft whenever the dialog opens with a different trigger. + useEffect(() => { + if (open) setDraft(draftFromTrigger(trigger)); + }, [open, trigger]); + + const handleSubmit = () => { + const labelTrimmed = draft.label.trim(); + + if (isEdit && trigger) { + // Build a PATCH body. Match the fields the backend accepts on + // PATCH /routine-triggers/:id (see updateRoutineTriggerSchema). + const patch: Record = { + label: labelTrimmed || null, + enabled: draft.enabled, + }; + if (trigger.kind === "schedule") { + patch.cronExpression = draft.cronExpression.trim(); + patch.timezone = trigger.timezone ?? fallbackTimezone; + } + if (trigger.kind === "webhook") { + patch.signingMode = draft.signingMode; + patch.replayWindowSec = parseReplayWindowSec(draft.replayWindowSec); + } + onSubmit({ id: trigger.id, kind: trigger.kind as TriggerKind, body: patch }); + return; + } + + // Create body: match POST /routines/:id/triggers (createRoutineTriggerSchema). + const body: Record = { + kind: draft.kind, + label: labelTrimmed || draft.kind, + }; + if (draft.kind === "schedule") { + body.cronExpression = draft.cronExpression.trim(); + body.timezone = fallbackTimezone; + } + if (draft.kind === "webhook") { + body.signingMode = draft.signingMode; + body.replayWindowSec = parseReplayWindowSec(draft.replayWindowSec); + } + onSubmit({ id: null, kind: draft.kind, body }); + }; + + const showWebhookFields = draft.kind === "webhook"; + const showScheduleFields = draft.kind === "schedule"; + + return ( + + + + {isEdit ? "Edit trigger" : "Add trigger"} + + Configure when and how this routine fires. + + + +
+
+ + setDraft((d) => ({ ...d, label: e.target.value }))} + /> +

+ Optional — shown in the trigger list. +

+
+ +
+ + + {isEdit && ( +

+ Kind can't be changed after creation. +

+ )} +
+ + {showScheduleFields && ( + setDraft((d) => ({ ...d, cronExpression }))} + /> + )} + + {showWebhookFields && ( +
+
+ + +

+ {signingModeDescriptions[draft.signingMode]} +

+
+ {!SIGNING_MODES_WITHOUT_REPLAY_WINDOW.has(draft.signingMode) && ( +
+ + + setDraft((d) => ({ ...d, replayWindowSec: e.target.value })) + } + /> +
+ )} +
+ )} +
+ + + {isEdit && ( + + )} + + + + + + ); +} diff --git a/ui/src/components/TriggerListCard.tsx b/ui/src/components/TriggerListCard.tsx new file mode 100644 index 00000000..6ca4b0ed --- /dev/null +++ b/ui/src/components/TriggerListCard.tsx @@ -0,0 +1,139 @@ +import { Clock3, Pencil, RefreshCw, Trash2, Webhook, Zap } from "lucide-react"; +import type { RoutineTrigger } from "@paperclipai/shared"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { ToggleSwitch } from "@/components/ui/toggle-switch"; +import { describeSchedule } from "./ScheduleEditor"; +import { timeAgo } from "../lib/timeAgo"; + +interface TriggerListCardProps { + trigger: RoutineTrigger; + onEdit: () => void; + onDelete: () => void; + onToggleEnabled: (enabled: boolean) => void; + onRotateSecret?: () => void; + togglePending?: boolean; +} + +export function TriggerListCard({ + trigger, + onEdit, + onDelete, + onToggleEnabled, + onRotateSecret, + togglePending, +}: TriggerListCardProps) { + const isSchedule = trigger.kind === "schedule"; + const isWebhook = trigger.kind === "webhook"; + const Icon = isSchedule ? Clock3 : isWebhook ? Webhook : Zap; + + const summary = isSchedule && trigger.cronExpression + ? describeSchedule(trigger.cronExpression) + : isWebhook + ? `Webhook${trigger.publicId ? ` · ${trigger.publicId}` : ""}` + : "API trigger"; + + const nextRun = isSchedule && trigger.enabled && trigger.nextRunAt + ? new Date(trigger.nextRunAt).toLocaleString(undefined, { + weekday: "short", + day: "numeric", + month: "short", + hour: "2-digit", + minute: "2-digit", + }) + : trigger.enabled ? "—" : "Disabled"; + + const lastFired = trigger.lastFiredAt ? timeAgo(trigger.lastFiredAt) : "Never"; + + const resultIsError = typeof trigger.lastResult === "string" && /error|fail/i.test(trigger.lastResult); + + return ( +
+
+
+ +
+ + {trigger.label || (isSchedule ? "Schedule" : isWebhook ? "Webhook" : "Trigger")} + + +
+ +
+ + {trigger.kind} + + {!trigger.enabled && ( + + paused + + )} +
+ +
{summary}
+ {isSchedule && trigger.cronExpression && ( +
+ {trigger.cronExpression} + {trigger.timezone ? ` · ${trigger.timezone}` : ""} +
+ )} + +
+
+
Next run
+
{nextRun}
+
+
+
Last fired
+
{lastFired}
+
+
+
Last result
+
+ {trigger.lastResult ? ( + + {trigger.lastResult} + + ) : ( + — + )} +
+
+
+ +
+ {isWebhook && onRotateSecret && ( + + )} + + +
+
+ ); +} diff --git a/ui/src/pages/RoutineDetail.tsx b/ui/src/pages/RoutineDetail.tsx index a8a0e9d0..076bf518 100644 --- a/ui/src/pages/RoutineDetail.tsx +++ b/ui/src/pages/RoutineDetail.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Link, useLocation, useNavigate, useParams } from "@/lib/router"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { @@ -9,15 +9,16 @@ import { Copy, History as HistoryIcon, Play, - RefreshCw, + Plus, Repeat, Save, - Trash2, - Webhook, - Zap, + SlidersHorizontal, } from "lucide-react"; import { ApiError } from "../api/client"; import { routinesApi, type RoutineTriggerResponse, type RotateRoutineTriggerResponse, type RestoreRoutineRevisionResponse } from "../api/routines"; +import { TriggerListCard } from "../components/TriggerListCard"; +import { TriggerDialog } from "../components/TriggerDialog"; +import { ConfirmDialog } from "../components/ConfirmDialog"; import { RoutineHistoryTab, type RoutineHistoryDirtyFieldDescriptor, @@ -29,9 +30,10 @@ import { projectsApi } from "../api/projects"; import { accessApi } from "../api/access"; import { useCompany } from "../context/CompanyContext"; import { useBreadcrumbs } from "../context/BreadcrumbContext"; +import { usePanel } from "../context/PanelContext"; import { useToastActions } from "../context/ToastContext"; +import { cn } from "../lib/utils"; import { queryKeys } from "../lib/queryKeys"; -import { buildRoutineTriggerPatch } from "../lib/routine-trigger-patch"; import { buildMarkdownMentionOptions } from "../lib/company-members"; import { timeAgo } from "../lib/timeAgo"; import { ToggleSwitch } from "@/components/ui/toggle-switch"; @@ -45,7 +47,6 @@ import { type RoutineRunDialogSubmitData, } from "../components/RoutineRunVariablesDialog"; import { RoutineVariablesEditor, RoutineVariablesHint } from "../components/RoutineVariablesEditor"; -import { ScheduleEditor, describeSchedule } from "../components/ScheduleEditor"; import { RunButton } from "../components/AgentActionButtons"; import { getRecentAssigneeIds, sortAgentsByRecency, trackRecentAssignee } from "../lib/recent-assignees"; import { getRecentProjectIds, trackRecentProject } from "../lib/recent-projects"; @@ -67,8 +68,6 @@ import type { RoutineDetail as RoutineDetailType, RoutineTrigger, RoutineVariabl const concurrencyPolicies = ["coalesce_if_active", "always_enqueue", "skip_if_active"]; const catchUpPolicies = ["skip_missed", "enqueue_missed_with_cap"]; -const triggerKinds = ["schedule", "webhook"]; -const signingModes = ["bearer", "hmac_sha256", "github_hmac", "none"]; const routineTabs = ["triggers", "runs", "activity", "history"] as const; const concurrencyPolicyDescriptions: Record = { coalesce_if_active: "Keep one follow-up run queued while an active run is still working.", @@ -79,13 +78,6 @@ const catchUpPolicyDescriptions: Record = { skip_missed: "Ignore schedule windows that were missed while the routine or scheduler was paused.", enqueue_missed_with_cap: "Catch up missed schedule windows in capped batches after recovery.", }; -const signingModeDescriptions: Record = { - bearer: "Expect a shared bearer token in the Authorization header.", - hmac_sha256: "Expect an HMAC SHA-256 signature over the request using the shared secret.", - github_hmac: "Accept GitHub-style X-Hub-Signature-256 header (HMAC over raw body, no timestamp).", - none: "No authentication — the webhook URL itself acts as a shared secret.", -}; -const SIGNING_MODES_WITHOUT_REPLAY_WINDOW = new Set(["github_hmac", "none"]); type RoutineTab = (typeof routineTabs)[number]; @@ -150,128 +142,6 @@ function buildRoutineMutationPayload(input: { }; } -function TriggerEditor({ - trigger, - onSave, - onRotate, - onDelete, -}: { - trigger: RoutineTrigger; - onSave: (id: string, patch: Record) => void; - onRotate: (id: string) => void; - onDelete: (id: string) => void; -}) { - const [draft, setDraft] = useState({ - label: trigger.label ?? "", - cronExpression: trigger.cronExpression ?? "", - signingMode: trigger.signingMode ?? "bearer", - replayWindowSec: String(trigger.replayWindowSec ?? 300), - }); - - useEffect(() => { - setDraft({ - label: trigger.label ?? "", - cronExpression: trigger.cronExpression ?? "", - signingMode: trigger.signingMode ?? "bearer", - replayWindowSec: String(trigger.replayWindowSec ?? 300), - }); - }, [trigger]); - - return ( -
-
-
- {trigger.kind === "schedule" ? : trigger.kind === "webhook" ? : } - {trigger.label ?? trigger.kind} -
- - {trigger.kind === "schedule" && trigger.nextRunAt - ? `Next: ${new Date(trigger.nextRunAt).toLocaleString()}` - : trigger.kind === "webhook" - ? "Webhook" - : "API"} - -
- -
-
- - setDraft((current) => ({ ...current, label: event.target.value }))} - /> -
- {trigger.kind === "schedule" && ( -
- - setDraft((current) => ({ ...current, cronExpression }))} - /> -
- )} - {trigger.kind === "webhook" && ( - <> -
- - -
- {!SIGNING_MODES_WITHOUT_REPLAY_WINDOW.has(draft.signingMode) && ( -
- - setDraft((current) => ({ ...current, replayWindowSec: event.target.value }))} - /> -
- )} - - )} -
- -
- {trigger.lastResult && Last: {trigger.lastResult}} -
- {trigger.kind === "webhook" && ( - - )} - - -
-
-
- ); -} - export function RoutineDetail() { const { routineId } = useParams<{ routineId: string }>(); const { selectedCompanyId } = useCompany(); @@ -280,6 +150,7 @@ export function RoutineDetail() { const navigate = useNavigate(); const location = useLocation(); const { pushToast } = useToastActions(); + const { openPanel, closePanel, panelVisible, setPanelVisible } = usePanel(); const hydratedRoutineIdRef = useRef(null); const titleInputRef = useRef(null); const descriptionEditorRef = useRef(null); @@ -289,12 +160,10 @@ export function RoutineDetail() { const [advancedOpen, setAdvancedOpen] = useState(false); const [saveConflict, setSaveConflict] = useState(false); const [runVariablesOpen, setRunVariablesOpen] = useState(false); - const [newTrigger, setNewTrigger] = useState({ - kind: "schedule", - cronExpression: "0 10 * * *", - signingMode: "bearer", - replayWindowSec: "300", - }); + const [triggerDialogOpen, setTriggerDialogOpen] = useState(false); + const [editingTrigger, setEditingTrigger] = useState(null); + const [triggerPendingDelete, setTriggerPendingDelete] = useState(null); + const [togglingTriggerId, setTogglingTriggerId] = useState(null); const [editDraft, setEditDraft] = useState<{ title: string; description: string; @@ -441,7 +310,7 @@ export function RoutineDetail() { } }; - const setActiveTab = (value: string) => { + const setActiveTab = useCallback((value: string) => { if (!routineId || !isRoutineTab(value)) return; const params = new URLSearchParams(location.search); if (value === "triggers") { @@ -457,7 +326,7 @@ export function RoutineDetail() { }, { replace: true }, ); - }; + }, [location.pathname, location.search, navigate, routineId]); const saveRoutine = useMutation({ mutationFn: () => { @@ -494,6 +363,11 @@ export function RoutineDetail() { }); }, }); + const saveRoutineRef = useRef(saveRoutine); + + useEffect(() => { + saveRoutineRef.current = saveRoutine; + }, [saveRoutine]); const runRoutine = useMutation({ mutationFn: (data?: RoutineRunDialogSubmitData) => @@ -552,24 +426,23 @@ export function RoutineDetail() { }); const createTrigger = useMutation({ - mutationFn: async (): Promise => { - const existingOfKind = (routine?.triggers ?? []).filter((t) => t.kind === newTrigger.kind).length; - const autoLabel = existingOfKind > 0 ? `${newTrigger.kind}-${existingOfKind + 1}` : newTrigger.kind; - return routinesApi.createTrigger(routineId!, { - kind: newTrigger.kind, - label: autoLabel, - ...(newTrigger.kind === "schedule" - ? { cronExpression: newTrigger.cronExpression.trim(), timezone: getLocalTimezone() } - : {}), - ...(newTrigger.kind === "webhook" - ? { - signingMode: newTrigger.signingMode, - replayWindowSec: Number(newTrigger.replayWindowSec || "300"), - } - : {}), - }); + mutationFn: async (body: Record): Promise => { + // Auto-label when the caller didn't provide one (e.g. dialog left the + // Label field blank). Keeps the existing "schedule-2"-style numbering + // behaviour so existing routines keep unique-ish labels. + const kind = String(body.kind ?? "schedule"); + const trimmedLabel = typeof body.label === "string" ? body.label.trim() : ""; + let finalLabel: string; + if (trimmedLabel.length > 0 && trimmedLabel !== kind) { + finalLabel = trimmedLabel; + } else { + const existingOfKind = (routine?.triggers ?? []).filter((t) => t.kind === kind).length; + finalLabel = existingOfKind > 0 ? `${kind}-${existingOfKind + 1}` : kind; + } + return routinesApi.createTrigger(routineId!, { ...body, label: finalLabel }); }, onSuccess: async (result) => { + setTriggerDialogOpen(false); if (result.secretMaterial) { setSecretMessage({ title: "Webhook trigger created", @@ -605,9 +478,10 @@ export function RoutineDetail() { onSuccess: async () => { pushToast({ title: "Trigger saved", - body: "The routine cadence update was saved.", tone: "success", }); + setTriggerDialogOpen(false); + setEditingTrigger(null); await Promise.all([ queryClient.invalidateQueries({ queryKey: queryKeys.routines.detail(routineId!) }), queryClient.invalidateQueries({ queryKey: queryKeys.routines.list(selectedCompanyId!) }), @@ -621,6 +495,9 @@ export function RoutineDetail() { tone: "error", }); }, + onSettled: () => { + setTogglingTriggerId(null); + }, }); const deleteTrigger = useMutation({ @@ -630,6 +507,7 @@ export function RoutineDetail() { title: "Trigger deleted", tone: "success", }); + setTriggerPendingDelete(null); await Promise.all([ queryClient.invalidateQueries({ queryKey: queryKeys.routines.detail(routineId!) }), queryClient.invalidateQueries({ queryKey: queryKeys.routines.list(selectedCompanyId!) }), @@ -710,6 +588,237 @@ export function RoutineDetail() { const currentAssignee = editDraft.assigneeAgentId ? agentById.get(editDraft.assigneeAgentId) ?? null : null; const currentProject = editDraft.projectId ? projectById.get(editDraft.projectId) ?? null : null; + const activityTabsPanel = useMemo(() => { + if (!routine) return null; + return ( + + + + + Triggers + + + + Runs + {hasLiveRun && } + + + + Activity + + + + History + + + + + + + {routine.triggers.length === 0 ? ( +
+

No triggers yet

+

+ Triggers fire this routine on a schedule or via webhook. +

+ +
+ ) : ( +
+ {routine.triggers.map((trigger) => ( + { + setEditingTrigger(trigger); + setTriggerDialogOpen(true); + }} + onDelete={() => setTriggerPendingDelete(trigger)} + onToggleEnabled={(enabled) => { + setTogglingTriggerId(trigger.id); + updateTrigger.mutate({ id: trigger.id, patch: { enabled } }); + }} + onRotateSecret={ + trigger.kind === "webhook" + ? () => rotateTrigger.mutate(trigger.id) + : undefined + } + togglePending={togglingTriggerId === trigger.id} + /> + ))} +
+ )} + + + + {hasLiveRun && activeIssueId && routine && ( + + )} + {(routineRuns ?? []).length === 0 ? ( +

No runs yet.

+ ) : ( +
+ {(routineRuns ?? []).map((run) => ( +
+
+ {run.source} + + {run.status.replaceAll("_", " ")} + +
+ {(run.trigger || run.linkedIssue) && ( +
+ {run.trigger && ( + {run.trigger.label ?? run.trigger.kind} + )} + {run.linkedIssue && ( + + {run.linkedIssue.identifier ?? run.linkedIssue.id.slice(0, 8)} + + )} +
+ )} + {timeAgo(run.triggeredAt)} +
+ ))} +
+ )} + + + + {(activity ?? []).length === 0 ? ( +

No activity yet.

+ ) : ( +
+ {(activity ?? []).map((event) => ( +
+ {event.action.replaceAll(".", " ")} + {event.details && Object.keys(event.details).length > 0 && ( +
+ {Object.entries(event.details).slice(0, 3).map(([key, value], i) => ( + + {i > 0 && ·} + {key.replaceAll("_", " ")}:{" "} + {formatActivityDetailValue(value)} + + ))} +
+ )} + {timeAgo(event.createdAt)} +
+ ))} +
+ )} + + + + { + if (routineDefaults) setEditDraft(routineDefaults); + }} + onSaveEdits={() => { + const currentSave = saveRoutineRef.current; + if (!currentSave.isPending && editDraft.title.trim()) { + currentSave.mutate(); + } + }} + agents={agentById} + projects={projectById} + onRestoreSecretMaterials={(response: RestoreRoutineRevisionResponse) => { + if (response.secretMaterials.length > 0) { + setSecretMessage({ + title: response.secretMaterials.length === 1 + ? "Webhook trigger restored" + : `${response.secretMaterials.length} webhook triggers restored`, + entries: response.secretMaterials.map((recreated) => ({ + webhookUrl: recreated.webhookUrl, + webhookSecret: recreated.webhookSecret, + })), + }); + } + }} + onRestored={(response: RestoreRoutineRevisionResponse) => { + setSaveConflict(false); + queryClient.setQueryData( + queryKeys.routines.detail(routineId!), + (prev) => + prev + ? { + ...prev, + ...response.routine, + latestRevisionId: response.revision.id, + latestRevisionNumber: response.revision.revisionNumber, + } + : prev, + ); + setEditDraft({ + title: response.routine.title, + description: response.routine.description ?? "", + projectId: response.routine.projectId ?? "", + assigneeAgentId: response.routine.assigneeAgentId ?? "", + priority: response.routine.priority, + concurrencyPolicy: response.routine.concurrencyPolicy, + catchUpPolicy: response.routine.catchUpPolicy, + variables: response.routine.variables, + }); + hydratedRoutineIdRef.current = response.routine.id; + }} + /> + + + ); + }, [ + activeIssueId, + activeTab, + activity, + agentById, + dirtyFields, + editDraft.title, + hasLiveRun, + isEditDirty, + projectById, + queryClient, + rotateTrigger.mutate, + routine, + routineDefaults, + routineRuns, + routineId, + setActiveTab, + togglingTriggerId, + updateTrigger.mutate, + ]); + + useEffect(() => { + if (!activityTabsPanel) { + closePanel(); + return; + } + openPanel(activityTabsPanel); + return () => closePanel(); + }, [activityTabsPanel, closePanel, openPanel]); + if (!selectedCompanyId) { return ; } @@ -811,6 +920,18 @@ export function RoutineDetail() { {automationLabel} +
@@ -1064,225 +1185,12 @@ export function RoutineDetail() { - + - {/* Tabs */} - - - - - Triggers - - - - Runs - {hasLiveRun && } - - - - Activity - - - - History - - - - - {/* Add trigger form */} -
-

Add trigger

-
-
- - -
- {newTrigger.kind === "schedule" && ( -
- - setNewTrigger((current) => ({ ...current, cronExpression }))} - /> -
- )} - {newTrigger.kind === "webhook" && ( - <> -
- - -

{signingModeDescriptions[newTrigger.signingMode]}

-
- {!SIGNING_MODES_WITHOUT_REPLAY_WINDOW.has(newTrigger.signingMode) && ( -
- - setNewTrigger((current) => ({ ...current, replayWindowSec: event.target.value }))} /> -
- )} - - )} -
-
- -
-
- - {/* Existing triggers */} - {routine.triggers.length === 0 ? ( -

No triggers configured yet.

- ) : ( -
- {routine.triggers.map((trigger) => ( - updateTrigger.mutate({ id, patch })} - onRotate={(id) => rotateTrigger.mutate(id)} - onDelete={(id) => deleteTrigger.mutate(id)} - /> - ))} -
- )} - - - - {hasLiveRun && activeIssueId && routine && ( - - )} - {(routineRuns ?? []).length === 0 ? ( -

No runs yet.

- ) : ( -
- {(routineRuns ?? []).map((run) => ( -
-
- {run.source} - - {run.status.replaceAll("_", " ")} - - {run.trigger && ( - {run.trigger.label ?? run.trigger.kind} - )} - {run.linkedIssue && ( - - {run.linkedIssue.identifier ?? run.linkedIssue.id.slice(0, 8)} - - )} -
- {timeAgo(run.triggeredAt)} -
- ))} -
- )} - - - - {(activity ?? []).length === 0 ? ( -

No activity yet.

- ) : ( -
- {(activity ?? []).map((event) => ( -
-
- {event.action.replaceAll(".", " ")} - {event.details && Object.keys(event.details).length > 0 && ( - - {Object.entries(event.details).slice(0, 3).map(([key, value], i) => ( - - {i > 0 && ·} - {key.replaceAll("_", " ")}:{" "} - {formatActivityDetailValue(value)} - - ))} - - )} -
- {timeAgo(event.createdAt)} -
- ))} -
- )} - - - - { - if (routineDefaults) setEditDraft(routineDefaults); - }} - onSaveEdits={() => { - if (!saveRoutine.isPending && editDraft.title.trim()) { - saveRoutine.mutate(); - } - }} - agents={agentById} - projects={projectById} - onRestoreSecretMaterials={(response: RestoreRoutineRevisionResponse) => { - if (response.secretMaterials.length > 0) { - setSecretMessage({ - title: response.secretMaterials.length === 1 - ? "Webhook trigger restored" - : `${response.secretMaterials.length} webhook triggers restored`, - entries: response.secretMaterials.map((recreated) => ({ - webhookUrl: recreated.webhookUrl, - webhookSecret: recreated.webhookSecret, - })), - }); - } - }} - onRestored={(response: RestoreRoutineRevisionResponse) => { - setSaveConflict(false); - queryClient.setQueryData( - queryKeys.routines.detail(routineId!), - (prev) => - prev - ? { - ...prev, - ...response.routine, - latestRevisionId: response.revision.id, - latestRevisionNumber: response.revision.revisionNumber, - } - : prev, - ); - setEditDraft({ - title: response.routine.title, - description: response.routine.description ?? "", - projectId: response.routine.projectId ?? "", - assigneeAgentId: response.routine.assigneeAgentId ?? "", - priority: response.routine.priority, - concurrencyPolicy: response.routine.concurrencyPolicy, - catchUpPolicy: response.routine.catchUpPolicy, - variables: response.routine.variables, - }); - hydratedRoutineIdRef.current = response.routine.id; - }} - /> - - + {/* Tabs (mobile only — desktop renders in the right properties panel) */} +
+ {activityTabsPanel} +
runRoutine.mutate(data)} /> + + { + setTriggerDialogOpen(next); + if (!next) setEditingTrigger(null); + }} + trigger={editingTrigger} + fallbackTimezone={getLocalTimezone()} + submitting={createTrigger.isPending || updateTrigger.isPending} + onSubmit={({ id, body }) => { + if (id) { + updateTrigger.mutate({ id, patch: body }); + } else { + createTrigger.mutate(body); + } + }} + /> + + { + if (!next) setTriggerPendingDelete(null); + }} + title="Delete trigger?" + description={ + triggerPendingDelete + ? `"${triggerPendingDelete.label ?? triggerPendingDelete.kind}" will be removed. This can't be undone.` + : undefined + } + confirmLabel="Delete" + destructive + busy={deleteTrigger.isPending} + onConfirm={() => { + if (triggerPendingDelete) deleteTrigger.mutate(triggerPendingDelete.id); + }} + /> ); }