mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-12 08:19:54 +02:00
## Thinking Path > - Paperclip is the open-source app people use to manage AI agents for work > - The `ui` package contains React components that drive the agent scheduling and workspace configuration UX > - Dependabot PR #9895 bumped `@radix-ui/react-*` from 1.6.0 → 1.6.4, which changed the internal effect-scheduling order inside Dialog and Select primitives > - Two `workspaces-a` CI tests started failing: `RoutineRunVariablesDialog` and `editable-sections / TriggersSection` > - Root cause for both: radix 1.6.4's changed scheduling pushes a cascaded state update one render-tick later, and each test asserted before that tick landed > - This PR fixes the two affected surfaces at the source (one production fix, one test fix) so the radix bump can land cleanly > - The benefit is unblocking PR #9895 without compromising test fidelity or production correctness ## Linked Issues or Issue Description Refs #9895 — this PR fixes the two `workspaces-a` test failures that blocked the radix-ui 1.6.0 → 1.6.4 Dependabot bump. **Root cause:** radix-ui 1.6.4 changed the internal effect-scheduling order inside its Dialog and Select primitives, pushing certain cascaded state updates one render-tick later than before. Two tests each asserted against the intermediate state, before the deferred tick landed. **Affected tests (both now pass at radix 1.6.0 AND 1.6.4):** 1. `editable-sections / TriggersSection` — `ScheduleEditor` disabled-button assertion 2. `RoutineRunVariablesDialog` — workspace branch propagation assertion Production behavior is unchanged and correct in both cases (verified via tracing). ## What Changed - **`ui/src/components/ScheduleEditor.tsx`** — `onValidityChange` is now called synchronously inside the custom-cron `onChange` handler, not only via the passive `useEffect` below. Previously there was a one-render window where an invalid draft still read as valid to the parent (button enabled); this closes that window. The effect call is preserved as a safety net for other entry paths; only the `onChange` path is new. - **`ui/src/components/RoutineRunVariablesDialog.test.tsx`** — the post-mount settle loop now waits for the branch value to actually appear in an `<input>` (`value === "pap-1634-routine-branch"`) rather than exiting as soon as the workspace card mounts. The card reports its branch name through an effect callback that triggers a follow-up render; the old loop exited one tick too early. Iteration cap raised from 10 → 20 to give the extra tick room. This PR intentionally does **not** bump radix-ui — that stays in #9895. ## Verification ```sh # TypeScript — clean at radix 1.6.0 (master): tsc -p ui/tsconfig.json # Targeted vitest: npx vitest run ui/src/components/RoutineRunVariablesDialog.test.tsx npx vitest run ui/src/components/editable-sections npx vitest run ui/src/components/ScheduleEditor # Full ui suite — green with radix 1.6.4 installed locally (371 files / 3035 tests): npx vitest run --project ui # check-forbidden-tokens — clean ``` All of the above pass at **both** radix 1.6.0 (current master) and 1.6.4. ## Risks Low risk. The production change (`ScheduleEditor.tsx`) adds a synchronous call to an already-injected `onValidityChange` prop — same value, earlier in the same event cycle. No new state, no new effects, no API changes. The test change tightens an assertion (waits longer, checks a more specific condition) rather than relaxing one. ## Model Used Claude Sonnet (Anthropic) — Paperclip agent workflow; model family claude-sonnet-4-x with tool use and extended reasoning enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Harold Kim <harold@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing>
418 lines
14 KiB
TypeScript
418 lines
14 KiB
TypeScript
import { useCallback, useEffect, useMemo, useState } from "react";
|
|
import { Button } from "@/components/ui/button";
|
|
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
|
|
import { Input } from "@/components/ui/input";
|
|
import { nextCronFires, parseCronExpression } from "../lib/cron-fires";
|
|
|
|
export type SchedulePreset = "every_minute" | "every_hour" | "every_day" | "weekdays" | "weekly" | "monthly" | "custom";
|
|
|
|
const PRESETS: { value: SchedulePreset; label: string }[] = [
|
|
{ value: "every_minute", label: "Every minute" },
|
|
{ value: "every_hour", label: "Every hour" },
|
|
{ value: "every_day", label: "Every day" },
|
|
{ value: "weekdays", label: "Weekdays" },
|
|
{ value: "weekly", label: "Weekly" },
|
|
{ value: "monthly", label: "Monthly" },
|
|
{ value: "custom", label: "Custom (cron)" },
|
|
];
|
|
|
|
const HOURS = Array.from({ length: 24 }, (_, i) => ({
|
|
value: String(i),
|
|
label: i === 0 ? "12 AM" : i < 12 ? `${i} AM` : i === 12 ? "12 PM" : `${i - 12} PM`,
|
|
}));
|
|
|
|
const MINUTES = Array.from({ length: 12 }, (_, i) => ({
|
|
value: String(i * 5),
|
|
label: String(i * 5).padStart(2, "0"),
|
|
}));
|
|
|
|
const DAYS_OF_WEEK = [
|
|
{ value: "1", label: "Mon" },
|
|
{ value: "2", label: "Tue" },
|
|
{ value: "3", label: "Wed" },
|
|
{ value: "4", label: "Thu" },
|
|
{ value: "5", label: "Fri" },
|
|
{ value: "6", label: "Sat" },
|
|
{ value: "0", label: "Sun" },
|
|
];
|
|
|
|
const DAYS_OF_MONTH = Array.from({ length: 31 }, (_, i) => ({
|
|
value: String(i + 1),
|
|
label: String(i + 1),
|
|
}));
|
|
|
|
function hasOption(options: Array<{ value: string }>, value: string): boolean {
|
|
return options.some((option) => option.value === value);
|
|
}
|
|
|
|
export function parseCronToPreset(cron: string): {
|
|
preset: SchedulePreset;
|
|
hour: string;
|
|
minute: string;
|
|
dayOfWeek: string;
|
|
dayOfMonth: string;
|
|
} {
|
|
const defaults = { hour: "10", minute: "0", dayOfWeek: "1", dayOfMonth: "1" };
|
|
|
|
if (!cron || !cron.trim()) {
|
|
return { preset: "every_day", ...defaults };
|
|
}
|
|
|
|
const parts = cron.trim().split(/\s+/);
|
|
if (parts.length !== 5) {
|
|
return { preset: "custom", ...defaults };
|
|
}
|
|
|
|
const [min, hr, dom, month, dow] = parts;
|
|
const selectableMinute = hasOption(MINUTES, min);
|
|
const selectableHour = hasOption(HOURS, hr);
|
|
|
|
// Every minute: "* * * * *"
|
|
if (min === "*" && hr === "*" && dom === "*" && month === "*" && dow === "*") {
|
|
return { preset: "every_minute", ...defaults };
|
|
}
|
|
|
|
// Every hour: "0 * * * *"
|
|
if (hr === "*" && dom === "*" && month === "*" && dow === "*" && selectableMinute) {
|
|
return { preset: "every_hour", ...defaults, minute: min };
|
|
}
|
|
|
|
// Every day: "M H * * *"
|
|
if (dom === "*" && month === "*" && dow === "*" && selectableHour && selectableMinute) {
|
|
return { preset: "every_day", ...defaults, hour: hr, minute: min };
|
|
}
|
|
|
|
// Weekdays: "M H * * 1-5"
|
|
if (dom === "*" && month === "*" && dow === "1-5" && selectableHour && selectableMinute) {
|
|
return { preset: "weekdays", ...defaults, hour: hr, minute: min };
|
|
}
|
|
|
|
// Weekly: "M H * * D" (single day)
|
|
if (dom === "*" && month === "*" && hasOption(DAYS_OF_WEEK, dow) && selectableHour && selectableMinute) {
|
|
return { preset: "weekly", ...defaults, hour: hr, minute: min, dayOfWeek: dow };
|
|
}
|
|
|
|
// Monthly: "M H D * *"
|
|
if (month === "*" && hasOption(DAYS_OF_MONTH, dom) && dow === "*" && selectableHour && selectableMinute) {
|
|
return { preset: "monthly", ...defaults, hour: hr, minute: min, dayOfMonth: dom };
|
|
}
|
|
|
|
return { preset: "custom", ...defaults };
|
|
}
|
|
|
|
export function buildCron(preset: SchedulePreset, hour: string, minute: string, dayOfWeek: string, dayOfMonth: string): string {
|
|
switch (preset) {
|
|
case "every_minute":
|
|
return "* * * * *";
|
|
case "every_hour":
|
|
return `${minute} * * * *`;
|
|
case "every_day":
|
|
return `${minute} ${hour} * * *`;
|
|
case "weekdays":
|
|
return `${minute} ${hour} * * 1-5`;
|
|
case "weekly":
|
|
return `${minute} ${hour} * * ${dayOfWeek}`;
|
|
case "monthly":
|
|
return `${minute} ${hour} ${dayOfMonth} * *`;
|
|
case "custom":
|
|
return "";
|
|
}
|
|
}
|
|
|
|
function describeSchedule(cron: string): string {
|
|
const { preset, hour, minute, dayOfWeek, dayOfMonth } = parseCronToPreset(cron);
|
|
const hourLabel = HOURS.find((h) => h.value === hour)?.label ?? `${hour}`;
|
|
const timeStr = `${hourLabel.replace(/ (AM|PM)$/, "")}:${minute.padStart(2, "0")} ${hourLabel.match(/(AM|PM)$/)?.[0] ?? ""}`;
|
|
|
|
switch (preset) {
|
|
case "every_minute":
|
|
return "Every minute";
|
|
case "every_hour":
|
|
return `Every hour at :${minute.padStart(2, "0")}`;
|
|
case "every_day":
|
|
return `Every day at ${timeStr}`;
|
|
case "weekdays":
|
|
return `Weekdays at ${timeStr}`;
|
|
case "weekly": {
|
|
const day = DAYS_OF_WEEK.find((d) => d.value === dayOfWeek)?.label ?? dayOfWeek;
|
|
return `Every ${day} at ${timeStr}`;
|
|
}
|
|
case "monthly":
|
|
return `Monthly on the ${dayOfMonth}${ordinalSuffix(Number(dayOfMonth))} at ${timeStr}`;
|
|
case "custom":
|
|
return cron || "No schedule set";
|
|
}
|
|
}
|
|
|
|
function ordinalSuffix(n: number): string {
|
|
const s = ["th", "st", "nd", "rd"];
|
|
const v = n % 100;
|
|
return s[(v - 20) % 10] || s[v] || s[0];
|
|
}
|
|
|
|
export { describeSchedule };
|
|
|
|
export function getScheduleCronValidation(cron: string): {
|
|
valid: boolean;
|
|
message: string;
|
|
nextFires: Date[];
|
|
} {
|
|
const trimmed = cron.trim();
|
|
if (!trimmed) {
|
|
return {
|
|
valid: false,
|
|
message: "Enter a 5-field cron expression.",
|
|
nextFires: [],
|
|
};
|
|
}
|
|
|
|
const fields = trimmed.split(/\s+/);
|
|
if (fields.length !== 5) {
|
|
return {
|
|
valid: false,
|
|
message: `Use exactly 5 fields; this has ${fields.length}.`,
|
|
nextFires: [],
|
|
};
|
|
}
|
|
|
|
if (!parseCronExpression(trimmed)) {
|
|
return {
|
|
valid: false,
|
|
message: "Cron fields must use valid numbers, ranges, lists, wildcards, or steps.",
|
|
nextFires: [],
|
|
};
|
|
}
|
|
|
|
const nextFires = nextCronFires(trimmed, 3, { timeZone: "UTC" });
|
|
return {
|
|
valid: true,
|
|
message: nextFires.length > 0 ? "Valid cron." : "Valid cron, but no upcoming fires were found.",
|
|
nextFires,
|
|
};
|
|
}
|
|
|
|
export function ScheduleEditor({
|
|
value,
|
|
onChange,
|
|
onValidityChange,
|
|
}: {
|
|
value: string;
|
|
onChange: (cron: string) => void;
|
|
onValidityChange?: (valid: boolean) => void;
|
|
}) {
|
|
const parsed = useMemo(() => parseCronToPreset(value), [value]);
|
|
const [preset, setPreset] = useState<SchedulePreset>(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 customValidation = useMemo(() => getScheduleCronValidation(customCron), [customCron]);
|
|
|
|
useEffect(() => {
|
|
onValidityChange?.(preset !== "custom" || customValidation.valid);
|
|
}, [customValidation.valid, onValidityChange, preset]);
|
|
|
|
// Sync from external value changes
|
|
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);
|
|
}, [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));
|
|
}
|
|
},
|
|
[onChange],
|
|
);
|
|
|
|
const handlePresetChange = (newPreset: SchedulePreset) => {
|
|
setPreset(newPreset);
|
|
if (newPreset === "custom") {
|
|
setCustomCron(value);
|
|
} else {
|
|
emitChange(newPreset, hour, minute, dayOfWeek, dayOfMonth, customCron);
|
|
}
|
|
};
|
|
|
|
return (
|
|
<div className="space-y-3">
|
|
<Select value={preset} onValueChange={(v) => handlePresetChange(v as SchedulePreset)}>
|
|
<SelectTrigger className="w-full" aria-label="Schedule frequency">
|
|
<SelectValue placeholder="Choose frequency..." />
|
|
</SelectTrigger>
|
|
<SelectContent>
|
|
{PRESETS.map((p) => (
|
|
<SelectItem key={p.value} value={p.value}>
|
|
{p.label}
|
|
</SelectItem>
|
|
))}
|
|
</SelectContent>
|
|
</Select>
|
|
|
|
{preset === "custom" ? (
|
|
<div className="space-y-1.5">
|
|
<Input
|
|
value={customCron}
|
|
onChange={(e) => {
|
|
const nextCron = e.target.value;
|
|
setCustomCron(nextCron);
|
|
// Report validity synchronously with the keystroke so consumers can gate
|
|
// their submit affordance in the same render. Relying solely on the
|
|
// effect below leaves a one-tick window where an invalid draft still
|
|
// reads as valid to the parent.
|
|
const nextValidation = getScheduleCronValidation(nextCron);
|
|
onValidityChange?.(nextValidation.valid);
|
|
if (nextValidation.valid) {
|
|
emitChange("custom", hour, minute, dayOfWeek, dayOfMonth, nextCron);
|
|
}
|
|
}}
|
|
placeholder="0 10 * * *"
|
|
aria-label="Cron expression"
|
|
aria-invalid={!customValidation.valid}
|
|
className="font-mono text-sm"
|
|
/>
|
|
<p className="text-xs text-muted-foreground">
|
|
Five fields: minute hour day-of-month month day-of-week
|
|
</p>
|
|
<p
|
|
className={customValidation.valid ? "text-xs text-muted-foreground" : "text-xs text-destructive"}
|
|
aria-live="polite"
|
|
>
|
|
{customValidation.message}
|
|
{customValidation.valid && customValidation.nextFires.length > 0
|
|
? ` Next: ${customValidation.nextFires.map((fire) => fire.toLocaleString()).join(", ")}.`
|
|
: null}
|
|
</p>
|
|
</div>
|
|
) : (
|
|
<div className="flex flex-wrap items-center gap-2">
|
|
{preset !== "every_minute" && preset !== "every_hour" && (
|
|
<>
|
|
<span className="text-sm text-muted-foreground">at</span>
|
|
<Select
|
|
value={hour}
|
|
onValueChange={(h) => {
|
|
setHour(h);
|
|
emitChange(preset, h, minute, dayOfWeek, dayOfMonth, customCron);
|
|
}}
|
|
>
|
|
<SelectTrigger className="w-(--sz-120px)">
|
|
<SelectValue />
|
|
</SelectTrigger>
|
|
<SelectContent>
|
|
{HOURS.map((h) => (
|
|
<SelectItem key={h.value} value={h.value}>
|
|
{h.label}
|
|
</SelectItem>
|
|
))}
|
|
</SelectContent>
|
|
</Select>
|
|
<span className="text-sm text-muted-foreground">:</span>
|
|
<Select
|
|
value={minute}
|
|
onValueChange={(m) => {
|
|
setMinute(m);
|
|
emitChange(preset, hour, m, dayOfWeek, dayOfMonth, customCron);
|
|
}}
|
|
>
|
|
<SelectTrigger className="w-(--sz-80px)">
|
|
<SelectValue />
|
|
</SelectTrigger>
|
|
<SelectContent>
|
|
{MINUTES.map((m) => (
|
|
<SelectItem key={m.value} value={m.value}>
|
|
{m.label}
|
|
</SelectItem>
|
|
))}
|
|
</SelectContent>
|
|
</Select>
|
|
</>
|
|
)}
|
|
|
|
{preset === "every_hour" && (
|
|
<>
|
|
<span className="text-sm text-muted-foreground">at minute</span>
|
|
<Select
|
|
value={minute}
|
|
onValueChange={(m) => {
|
|
setMinute(m);
|
|
emitChange(preset, hour, m, dayOfWeek, dayOfMonth, customCron);
|
|
}}
|
|
>
|
|
<SelectTrigger className="w-(--sz-80px)">
|
|
<SelectValue />
|
|
</SelectTrigger>
|
|
<SelectContent>
|
|
{MINUTES.map((m) => (
|
|
<SelectItem key={m.value} value={m.value}>
|
|
:{m.label}
|
|
</SelectItem>
|
|
))}
|
|
</SelectContent>
|
|
</Select>
|
|
</>
|
|
)}
|
|
|
|
{preset === "weekly" && (
|
|
<>
|
|
<span className="text-sm text-muted-foreground">on</span>
|
|
<div className="flex gap-1">
|
|
{DAYS_OF_WEEK.map((d) => (
|
|
<Button
|
|
key={d.value}
|
|
type="button"
|
|
variant={dayOfWeek === d.value ? "default" : "outline"}
|
|
size="sm"
|
|
className="h-7 px-2 text-xs"
|
|
aria-pressed={dayOfWeek === d.value}
|
|
onClick={() => {
|
|
setDayOfWeek(d.value);
|
|
emitChange(preset, hour, minute, d.value, dayOfMonth, customCron);
|
|
}}
|
|
>
|
|
{d.label}
|
|
</Button>
|
|
))}
|
|
</div>
|
|
</>
|
|
)}
|
|
|
|
{preset === "monthly" && (
|
|
<>
|
|
<span className="text-sm text-muted-foreground">on day</span>
|
|
<Select
|
|
value={dayOfMonth}
|
|
onValueChange={(dom) => {
|
|
setDayOfMonth(dom);
|
|
emitChange(preset, hour, minute, dayOfWeek, dom, customCron);
|
|
}}
|
|
>
|
|
<SelectTrigger className="w-(--sz-80px)">
|
|
<SelectValue />
|
|
</SelectTrigger>
|
|
<SelectContent>
|
|
{DAYS_OF_MONTH.map((d) => (
|
|
<SelectItem key={d.value} value={d.value}>
|
|
{d.label}
|
|
</SelectItem>
|
|
))}
|
|
</SelectContent>
|
|
</Select>
|
|
</>
|
|
)}
|
|
</div>
|
|
)}
|
|
</div>
|
|
);
|
|
}
|