diff --git a/docs/plans/goose-schedules-in-berd.md b/docs/plans/goose-schedules-in-berd.md index a4e3691e1..e56c1aa3c 100644 --- a/docs/plans/goose-schedules-in-berd.md +++ b/docs/plans/goose-schedules-in-berd.md @@ -11,11 +11,11 @@ The current Berd pin already includes typed ACP schedule list, update, pause/res | Layer | Scope | Acceptance | |---|---|---| | 1. `feat(automations): show local Goose schedules in OSS` | Give local schedules an OSS capability independent of the KGoose-gated company Automations capability; mount a read-only local schedule list in the shared hub; keep company-only details, history, builder, and requests gated. | With company automations disabled, list local jobs and status, and prove that company list/detail requests are not made. In the Block build, both sources remain visible in the same hub. | -| 2. `feat(automations): configure local Goose schedules` | Add guarded cron editing to the embedded schedule list, with supported-expression validation and save/cancel states. | Invalid input is rejected before reaching ACP, and valid updates use the typed update method against the live scheduler. The current Goose update handler mutates the active job before validating the new cron, so the UI must not pass unsupported expressions. | +| 2. `feat(automations): configure local Goose schedules` | Add cron editing to the embedded schedule list with explicit save/cancel, conservative validation, and error states that account for the pinned backend's update ordering. | Invalid or unsupported input is rejected before ACP; valid edits use the typed update method. Failed updates clearly warn that the current Goose backend may have removed the prior timer. | | 3. `feat(automations): manage local Goose schedules` | Add pause/resume, stop active runs, confirmed removal, and bounded per-schedule run history on demand. Share the local ACP adapter between UI and `berdctl` if/when CLI controls are restacked. | Mutations use typed ACP on the same live Goose process; enforce running-job restrictions; removal preserves the recipe; history is fetched only when requested and remains bounded. Review deletion behavior against the exact Goose version before adding any multi-step pause/delete/rollback sequence. | | Future upstream dependency | Add durable project/workspace context to scheduled jobs and pass it into every run's session creation. Then expose project filtering/association in Berd. | Schedule association survives restart and each execution uses the selected project's stable ID and workspace roots. | -Each proposed layer should be reviewed as a working prefix. The current first implementation covers only Layer 1; the later controls and upstream project-scoped execution work are planned separately because they carry distinct behavioral risks. +Each proposed layer should be reviewed as a working prefix. Layer 1 exposes read-only schedules. Layer 2 allows guarded cron updates but cannot guarantee the previous timer survives an unexpected backend rejection; failed updates warn users to verify the schedule and cannot be retried unchanged because Goose's same-expression shortcut may falsely report recovery. Pause/resume, stop, removal, run history, and upstream project-scoped execution remain separate follow-on work. ## Project association diff --git a/src/features/automations/api/localSchedules.test.ts b/src/features/automations/api/localSchedules.test.ts index c4c3fdf54..ad3c23737 100644 --- a/src/features/automations/api/localSchedules.test.ts +++ b/src/features/automations/api/localSchedules.test.ts @@ -1,18 +1,31 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { getClient } from "@/shared/api/acpConnection"; -import { listLocalSchedules } from "./localSchedules"; +import { listLocalSchedules, updateLocalScheduleCron } from "./localSchedules"; const listSchedules = vi.hoisted(() => vi.fn()); +const updateSchedule = vi.hoisted(() => vi.fn()); vi.mock("@/shared/api/acpConnection", () => ({ getClient: vi.fn(async () => ({ - goose: { GooseUnstableSchedulesList: listSchedules }, + goose: { + GooseUnstableSchedulesList: listSchedules, + GooseUnstableSchedulesUpdate: updateSchedule, + }, })), })); describe("local Goose schedules API", () => { beforeEach(() => { vi.clearAllMocks(); + updateSchedule.mockResolvedValue({ + job: { + id: "weekday-report", + source: "/tmp/scheduled_recipes/weekday-report.yaml", + cron: "0 10 * * 1-5", + currentlyRunning: false, + paused: true, + }, + }); listSchedules.mockResolvedValue({ jobs: [ { @@ -39,4 +52,14 @@ describe("local Goose schedules API", () => { expect(getClient).toHaveBeenCalledOnce(); expect(listSchedules).toHaveBeenCalledWith({}); }); + + it("updates a schedule through the connected Goose instance", async () => { + await expect( + updateLocalScheduleCron("weekday-report", "0 10 * * 1-5"), + ).resolves.toMatchObject({ cron: "0 10 * * 1-5", paused: true }); + expect(updateSchedule).toHaveBeenCalledWith({ + scheduleId: "weekday-report", + cron: "0 10 * * 1-5", + }); + }); }); diff --git a/src/features/automations/api/localSchedules.ts b/src/features/automations/api/localSchedules.ts index bb614def3..bc7d27f77 100644 --- a/src/features/automations/api/localSchedules.ts +++ b/src/features/automations/api/localSchedules.ts @@ -9,3 +9,15 @@ export async function listLocalSchedules(): Promise { const response = await client.goose.GooseUnstableSchedulesList({}); return response.jobs; } + +export async function updateLocalScheduleCron( + scheduleId: string, + cron: string, +): Promise { + const client = await getClient(); + const response = await client.goose.GooseUnstableSchedulesUpdate({ + scheduleId, + cron, + }); + return response.job; +} diff --git a/src/features/automations/lib/localScheduleCron.test.ts b/src/features/automations/lib/localScheduleCron.test.ts new file mode 100644 index 000000000..a7f8064d5 --- /dev/null +++ b/src/features/automations/lib/localScheduleCron.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { validateLocalScheduleCron } from "./localScheduleCron"; + +describe("validateLocalScheduleCron", () => { + it.each([ + "0 9 * * 1-5", + "*/15 * * * *", + "0,30 */5 8-17 * * 0,7", + "*/10 * * * *", + "0 0 0 28 * *", + "0 0 0 */1 * *", + "*/10 * * * * *", + "0 0 9 * * MON", + " 0 9 * * 1-5 ", + ])("accepts supported cron expression %s", (cron) => { + expect(validateLocalScheduleCron(cron)).toBeNull(); + }); + + it.each([ + "", + "@daily", + "0 0 9 * * * 2028", + "60 * * * *", + "0 24 * * *", + "0 0 0 32 * *", + "0 0 0 1 FEB *", + "0 0 0 * 13 *", + "0 0 0 * * 8", + "0 0 0 * * 5-2", + "0 0 9 1 */1 MON", + "0 0 0 * * */0", + "0 0 0 * * 1,,2", + "0 0 0 * * 1/999", + "0 0 0 L * *", + "*,15 9 * * *", + "*/5,15 9 * * *", + "0 0 0 1 * MON", + "0 0 0 * * 1#2", + "0 9 * * 1-5\uFEFF", + ])("rejects expressions outside the safe subset: %s", (cron) => { + expect(validateLocalScheduleCron(cron)).not.toBeNull(); + }); +}); diff --git a/src/features/automations/lib/localScheduleCron.ts b/src/features/automations/lib/localScheduleCron.ts new file mode 100644 index 000000000..0239daad5 --- /dev/null +++ b/src/features/automations/lib/localScheduleCron.ts @@ -0,0 +1,150 @@ +const FIELD_BOUNDS = [ + [0, 59], + [0, 59], + [0, 23], + [1, 31], + [1, 12], + [0, 7], +] as const; + +const MONTH_NAMES: Record = { + JAN: 1, + FEB: 2, + MAR: 3, + APR: 4, + MAY: 5, + JUN: 6, + JUL: 7, + AUG: 8, + SEP: 9, + OCT: 10, + NOV: 11, + DEC: 12, +}; + +const WEEKDAY_NAMES: Record = { + SUN: 0, + MON: 1, + TUE: 2, + WED: 3, + THU: 4, + FRI: 5, + SAT: 6, +}; + +const MONTH_ALIASES = new RegExp( + `\\b(${Object.keys(MONTH_NAMES).join("|")})\\b`, + "g", +); +const WEEKDAY_ALIASES = new RegExp( + `\\b(${Object.keys(WEEKDAY_NAMES).join("|")})\\b`, + "g", +); + +function resolveValue(value: string, fieldIndex: number): number | null { + const upperValue = value.toUpperCase(); + const resolved = + fieldIndex === 4 + ? MONTH_NAMES[upperValue] + : fieldIndex === 5 + ? WEEKDAY_NAMES[upperValue] + : undefined; + if (resolved !== undefined) return resolved; + if (!/^\d+$/.test(value)) return null; + const parsed = Number(value); + return Number.isSafeInteger(parsed) ? parsed : null; +} + +function isFullWildcard(field: string): boolean { + // Goose only marks a literal `*` as wildcard for day-of-month/weekday + // matching; `*/1` is parsed as all values but still combined with AND. + return field === "*"; +} + +function isValidCronField(field: string, fieldIndex: number): boolean { + const [minimum, maximum] = FIELD_BOUNDS[fieldIndex]; + if (field.length === 0) return false; + const items = field.split(","); + + for (const item of items) { + if (!item) return false; + + const steppedParts = item.split("/"); + if (steppedParts.length > 2) return false; + const [base, stepText] = steppedParts; + if (stepText !== undefined) { + const step = resolveValue(stepText, fieldIndex); + if (step === null || step < 1 || step > maximum - minimum + 1) { + return false; + } + } + + if (base === "*") { + if (items.length !== 1) return false; + continue; + } + + const rangeParts = base.split("-"); + if (rangeParts.length > 2) return false; + const start = resolveValue(rangeParts[0], fieldIndex); + const end = + rangeParts.length === 2 ? resolveValue(rangeParts[1], fieldIndex) : start; + if ( + start === null || + end === null || + start < minimum || + end > maximum || + start > end + ) { + return false; + } + } + + return true; +} + +/** + * Checks Berd's deliberately conservative editable cron subset before calling + * the pinned Goose scheduler, whose update path removes the old timer before + * parsing the replacement. Existing expressions outside this subset remain + * visible and can still be saved unchanged. The selected ruleset also excludes + * expressions whose recurrence semantics are ambiguous or infrequent. + */ +export function validateLocalScheduleCron(cron: string): string | null { + if ([...cron].some((character) => character.charCodeAt(0) > 127)) { + return "unsupported"; + } + const trimmedCron = cron.trim(); + if (!trimmedCron) return "unsupported"; + const fields = trimmedCron.split(/[ \t\r\n\v\f]+/); + if (fields.length !== 5 && fields.length !== 6) { + return "fieldCount"; + } + + const normalized = fields.join(" ").toUpperCase(); + const canonical = normalized + .replace(MONTH_ALIASES, (name) => String(MONTH_NAMES[name])) + .replace(WEEKDAY_ALIASES, (name) => String(WEEKDAY_NAMES[name])); + const canonicalFields = canonical.split(" "); + const fieldsWithSeconds = + canonicalFields.length === 5 ? ["0", ...canonicalFields] : canonicalFields; + + if ( + !fieldsWithSeconds.every((field, index) => isValidCronField(field, index)) + ) { + return "unsupported"; + } + + // Keep edits to frequent, unambiguous recurrences: no month restriction, + // and no simultaneous day-of-month + weekday restriction (Goose combines + // those with AND). + if ( + !isFullWildcard(fieldsWithSeconds[4]) || + (!isFullWildcard(fieldsWithSeconds[3]) && + !isFullWildcard(fieldsWithSeconds[5])) + ) { + return "safeWindow"; + } + + return null; +} diff --git a/src/features/automations/lib/localScheduleSpanish.test.ts b/src/features/automations/lib/localScheduleSpanish.test.ts new file mode 100644 index 000000000..5e813bb13 --- /dev/null +++ b/src/features/automations/lib/localScheduleSpanish.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import esAutomations from "@/shared/i18n/locales/es/automations.json"; + +describe("Spanish local schedule copy", () => { + it.each([ + [ + "loadErrorDescription", + "El programador local no está disponible o Goose no está conectado.", + ], + [ + "refreshError", + "No se pudieron actualizar las programaciones: {{error}}. Se muestra la última lista cargada.", + ], + ["emptyTitle", "Aún no hay programaciones locales"], + [ + "emptyBody", + "Las programaciones que crees en Goose aparecerán aquí. Usa la CLI de Goose u otro cliente de Goose para programar y configurar recetas.", + ], + ["running", "En ejecución"], + ["lastRun", "Última ejecución: {{time}}"], + ["recoverCron", "Recuperar la programación de {{id}}"], + ["recoverAction", "Recuperar programación"], + ["uncertainTitle", "Es necesario verificar el temporizador"], + ["editCron", "Editar la expresión cron de {{id}}"], + ["cronLabel", "Expresión cron de {{id}}"], + ["reconcilingSchedule", "Comprobando el estado actual de la programación…"], + ["updateError", "No se pudo actualizar la programación de Goose."], + [ + "updateFailureCaution", + "Es posible que el programador haya quitado el temporizador anterior si la actualización falló inesperadamente. Verifica la programación en Goose antes de depender de ella.", + ], + [ + "refreshAfterUpdateError", + "La actualización de la lista también falló, así que se desconoce el estado actual de la programación.", + ], + [ + "failedUpdateRetry", + "Esta misma expresión ya falló. No la reintentes: es posible que Goose haya quitado el temporizador anterior. Verifica la programación o prueba otra expresión válida.", + ], + [ + "changedElsewhere", + "Esta programación cambió mientras la editabas. Cancela y vuelve a abrirla para revisar el valor más reciente.", + ], + ] as const)("preserves accented copy for %s", (key, expected) => { + expect(esAutomations.localSchedules[key]).toBe(expected); + }); + + it("preserves accented cron guidance", () => { + for (const word of [ + "día-del-mes", + "día-de-la-semana", + "numéricos", + "días", + "más", + "comodín", + "podría", + "actualización", + "programación", + "después", + ]) { + expect(esAutomations.localSchedules.cronHelp).toContain(word); + } + }); + + it.each([ + ["fieldCount", "Ingresa una expresión cron de cinco o seis campos."], + [ + "unsupported", + "La expresión no es válida o usa una sintaxis que este editor no puede actualizar de forma segura. La programación existente no cambió.", + ], + [ + "safeWindow", + "Usa un comodín para el mes y un comodín en día-del-mes o día-de-la-semana. Goose exige que ambos campos de día coincidan si ninguno es comodín y los combina con AND.", + ], + ] as const)("preserves accented validation for %s", (key, expected) => { + expect(esAutomations.localSchedules.validation[key]).toBe(expected); + }); + + it("retains the active and unknown last-run labels", () => { + expect(esAutomations.localSchedules.active).toBe("Activa"); + expect(esAutomations.localSchedules.unknownLastRun).toBe("Desconocida"); + }); +}); diff --git a/src/features/automations/ui/LocalScheduleCronEditor.tsx b/src/features/automations/ui/LocalScheduleCronEditor.tsx new file mode 100644 index 000000000..f46bd5199 --- /dev/null +++ b/src/features/automations/ui/LocalScheduleCronEditor.tsx @@ -0,0 +1,300 @@ +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { + useIsMutating, + useMutation, + useQuery, + useQueryClient, +} from "@tanstack/react-query"; +import { useTranslation } from "react-i18next"; +import { + IconAlertTriangle, + IconCheck, + IconPencil, + IconX, +} from "@tabler/icons-react"; +import type { ScheduledJobDto } from "@aaif/goose-sdk"; +import { + LOCAL_SCHEDULES_QUERY_KEY, + listLocalSchedules, + updateLocalScheduleCron, +} from "@/features/automations/api/localSchedules"; +import { validateLocalScheduleCron } from "@/features/automations/lib/localScheduleCron"; +import { formatAcpErrorMessage } from "@/shared/api/acpErrors"; +import { Alert, AlertDescription, AlertTitle } from "@/shared/ui/alert"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; + +interface LocalScheduleCronEditorProps { + schedule: ScheduledJobDto; +} + +export function LocalScheduleCronEditor({ + schedule, +}: LocalScheduleCronEditorProps) { + const { t } = useTranslation("automations"); + const queryClient = useQueryClient(); + const [isEditing, setIsEditing] = useState(false); + const [draft, setDraft] = useState(schedule.cron); + const [baselineCron, setBaselineCron] = useState(schedule.cron); + const [error, setError] = useState(null); + const [isSaving, setIsSaving] = useState(false); + const [isReconciling, setIsReconciling] = useState(false); + const editButtonRef = useRef(null); + const inputRef = useRef(null); + const editorRef = useRef(null); + const wasEditingRef = useRef(false); + const restoreFocusAfterEditRef = useRef(false); + // Keep known timer uncertainty across panel navigation for this app session. + // A list refresh cannot establish timer health; only a successful update can. + const failureQueryKey = ["local-goose-schedule-failures", schedule.id]; + const { data: failedCrons } = useQuery({ + queryKey: failureQueryKey, + queryFn: () => [], + enabled: false, + initialData: [], + gcTime: Infinity, + }); + const mutationKey = ["local-goose-schedule-update", schedule.id]; + const isScheduleMutating = useIsMutating({ mutationKey }) > 0; + const normalizedCron = draft.trim().replace(/[ \t\r\n\v\f]+/g, " "); + const validationError = validateLocalScheduleCron(draft); + const hasChanged = draft.trim() !== baselineCron.trim(); + const hasKnownCron = draft.trim() === schedule.cron.trim(); + const sameAsFailedCron = failedCrons.includes(normalizedCron); + const errorCoversCurrentDraft = error !== null && sameAsFailedCron; + + const updateMutation = useMutation({ + mutationKey, + mutationFn: ({ scheduleId, cron }: { scheduleId: string; cron: string }) => + updateLocalScheduleCron(scheduleId, cron), + onMutate: async () => { + setIsSaving(true); + await queryClient.cancelQueries({ queryKey: LOCAL_SCHEDULES_QUERY_KEY }); + }, + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + }); + setError(null); + queryClient.setQueryData(failureQueryKey, []); + setIsSaving(false); + restoreFocusAfterEditRef.current = + editorRef.current?.contains(document.activeElement) === true; + setIsEditing(false); + }, + onError: async (mutationError, variables) => { + setError( + formatAcpErrorMessage(mutationError, t("localSchedules.updateError")), + ); + queryClient.setQueryData(failureQueryKey, (previous = []) => [ + ...new Set([...previous, variables.cron]), + ]); + setIsSaving(false); + setIsReconciling(true); + restoreFocusAfterEditRef.current = + editorRef.current?.contains(document.activeElement) === true || + document.activeElement === document.body; + + try { + await queryClient.cancelQueries({ + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + }); + // Use the query's cancellation ordering instead of writing a detached + // whole-list snapshot. Later updates cancel this read, including across + // rows and remounts, so its result cannot overwrite newer schedule data. + await queryClient.fetchQuery({ + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + queryFn: listLocalSchedules, + staleTime: 0, + }); + } catch { + setError((current) => + current + ? `${current} ${t("localSchedules.refreshAfterUpdateError")}` + : t("localSchedules.refreshAfterUpdateError"), + ); + } finally { + setIsReconciling(false); + } + }, + }); + + const scheduleChangedElsewhere = + isEditing && + !updateMutation.isError && + !error && + schedule.cron !== baselineCron; + + const displayedError = + error && errorCoversCurrentDraft + ? error + : scheduleChangedElsewhere + ? t("localSchedules.changedElsewhere") + : validationError && !hasKnownCron + ? t(`localSchedules.validation.${validationError}`) + : isReconciling + ? null + : (error ?? + (sameAsFailedCron + ? t("localSchedules.failedUpdateRetry") + : null)); + const helpId = `local-schedule-cron-help-${schedule.id}`; + const errorId = `local-schedule-cron-error-${schedule.id}`; + const uncertaintyWarning = + failedCrons.length > 0 ? ( + + + ) : null; + + useLayoutEffect(() => { + if (isEditing) { + inputRef.current?.focus(); + } else if (wasEditingRef.current && restoreFocusAfterEditRef.current) { + editButtonRef.current?.focus(); + } + wasEditingRef.current = isEditing; + }, [isEditing]); + + useEffect(() => { + if (error && document.activeElement === document.body) { + inputRef.current?.focus(); + } + }, [error]); + + useEffect(() => { + if (!isEditing) { + setDraft(schedule.cron); + setBaselineCron(schedule.cron); + } + }, [isEditing, schedule.cron]); + + function beginEditing() { + restoreFocusAfterEditRef.current = true; + setBaselineCron(schedule.cron); + setDraft(schedule.cron); + setError(null); + setIsEditing(true); + } + + function cancelEditing() { + setBaselineCron(schedule.cron); + setDraft(schedule.cron); + setError(null); + setIsEditing(false); + } + + function save() { + if ( + isScheduleMutating || + (isReconciling && !errorCoversCurrentDraft) || + !hasChanged || + (validationError && !hasKnownCron) || + sameAsFailedCron || + scheduleChangedElsewhere || + schedule.currentlyRunning + ) { + return; + } + setError(null); + updateMutation.mutate({ scheduleId: schedule.id, cron: normalizedCron }); + } + + if (!isEditing) { + return ( +
0 ? "w-full space-y-2" : undefined}> + {uncertaintyWarning} + +
+ ); + } + + return ( +
+ {uncertaintyWarning} + + { + setDraft(event.target.value); + }} + aria-invalid={displayedError !== null} + aria-describedby={displayedError ? `${helpId} ${errorId}` : helpId} + disabled={isSaving} + autoComplete="off" + spellCheck={false} + className="font-mono" + /> +

+ {t("localSchedules.cronHelp")} +

+
+ {displayedError ? ( +

+ {displayedError} +

+ ) : isReconciling ? ( +

+ {t("localSchedules.reconcilingSchedule")} +

+ ) : null} +
+
+ + +
+
+ ); +} diff --git a/src/features/automations/ui/LocalSchedulesPanel.test.tsx b/src/features/automations/ui/LocalSchedulesPanel.test.tsx index 1d07f0202..0a643241c 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.test.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.test.tsx @@ -1,20 +1,43 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen } from "@testing-library/react"; +import { + act, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { formatDate } from "@/shared/i18n"; -import { listLocalSchedules } from "@/features/automations/api/localSchedules"; +import { + listLocalSchedules, + updateLocalScheduleCron, +} from "@/features/automations/api/localSchedules"; import { LocalSchedulesPanel } from "./LocalSchedulesPanel"; vi.mock("@/features/automations/api/localSchedules", () => ({ LOCAL_SCHEDULES_QUERY_KEY: ["local-goose-schedules"], LOCAL_SCHEDULES_REFETCH_INTERVAL_MS: 15_000, listLocalSchedules: vi.fn(), + updateLocalScheduleCron: vi.fn(), })); -function renderPanel() { - const queryClient = new QueryClient({ +function deferred() { + let resolvePromise: (value: T) => void = () => { + throw new Error("Deferred promise was resolved before initialization"); + }; + const promise = new Promise((resolve) => { + resolvePromise = resolve; + }); + return { promise, resolve: resolvePromise }; +} + +function createQueryClient() { + return new QueryClient({ defaultOptions: { queries: { retry: false } }, }); +} + +function renderPanel(queryClient = createQueryClient()) { return render( @@ -32,7 +55,7 @@ const schedule = { describe("LocalSchedulesPanel", () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); vi.mocked(listLocalSchedules).mockResolvedValue([schedule]); }); @@ -90,6 +113,465 @@ describe("LocalSchedulesPanel", () => { ).toBeInTheDocument(); }); + it("saves valid cron edits and refreshes the Goose schedule list", async () => { + vi.mocked(updateLocalScheduleCron).mockResolvedValue({ + ...schedule, + cron: "0 10 * * 1-5", + }); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockResolvedValueOnce([{ ...schedule, cron: "0 10 * * 1-5" }]); + renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + expect(input).toHaveValue("0 9 * * 1-5"); + + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => + expect(updateLocalScheduleCron).toHaveBeenCalledWith( + "weekday-report", + "0 10 * * 1-5", + ), + ); + await waitFor(() => + expect(screen.getByText("0 10 * * 1-5")).toBeInTheDocument(), + ); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + }); + + it("keeps the latest list response when a mutation resolves after a newer poll", async () => { + const mutation = deferred(); + const newerPoll = deferred<(typeof schedule)[]>(); + vi.mocked(updateLocalScheduleCron).mockReturnValue(mutation.promise); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(newerPoll.promise) + .mockResolvedValueOnce([ + { ...schedule, cron: "0 11 * * 1-5", paused: true }, + ]); + const { unmount } = renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(updateLocalScheduleCron).toHaveBeenCalledOnce()); + + fireEvent.click(screen.getByRole("button", { name: "Refresh" })); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + await act(async () => { + newerPoll.resolve([{ ...schedule, cron: "0 11 * * 1-5", paused: true }]); + await newerPoll.promise; + }); + await waitFor(() => + expect(screen.getByText("0 11 * * 1-5")).toBeInTheDocument(), + ); + await act(async () => { + mutation.resolve({ ...schedule, cron: "0 10 * * 1-5" }); + await mutation.promise; + }); + await waitFor(() => expect(updateLocalScheduleCron).toHaveBeenCalledOnce()); + expect(screen.getByText("0 11 * * 1-5")).toBeInTheDocument(); + expect(screen.getByText("Paused")).toBeInTheDocument(); + unmount(); + }); + + it("cancels an edit without calling the scheduler", async () => { + renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + + expect(updateLocalScheduleCron).not.toHaveBeenCalled(); + expect(screen.getByText("0 9 * * 1-5")).toBeInTheDocument(); + }); + + it("moves focus into and out of the cron editor", async () => { + renderPanel(); + await screen.findByText("weekday-report"); + + const editButton = screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }); + fireEvent.click(editButton); + const input = screen.getByLabelText("Cron expression for weekday-report"); + await waitFor(() => expect(input).toHaveFocus()); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + await waitFor(() => + expect( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ).toHaveFocus(), + ); + }); + + it("prevents unsupported cron edits from reaching Goose", async () => { + renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 0 9 1 * MON" } }); + + expect(input).toHaveAttribute("aria-invalid", "true"); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("Use a wildcard for month"), + ); + expect( + screen.getByText(/^Use a wildcard for month/).closest('[role="status"]'), + ).toHaveAttribute("aria-live", "polite"); + expect(screen.getByText(/^Use a wildcard for month/)).toHaveTextContent( + "Use a wildcard for month and a wildcard in either day-of-month or weekday", + ); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + expect(updateLocalScheduleCron).not.toHaveBeenCalled(); + }); + + it("does not mark an unchanged unsupported existing cron invalid without an error", async () => { + vi.mocked(listLocalSchedules).mockResolvedValue([ + { ...schedule, cron: "0 0 1 1 *" }, + ]); + renderPanel(); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + expect(input).toHaveAttribute("aria-invalid", "false"); + expect(input).toHaveAttribute( + "aria-describedby", + "local-schedule-cron-help-weekday-report", + ); + expect( + screen.queryByText(/^Use a wildcard for month/), + ).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + }); + + it("keeps timer uncertainty after reconciliation, cancel, navigation, and refresh until recovery succeeds", async () => { + const reconciliation = deferred<(typeof schedule)[]>(); + const failed = { ...schedule, cron: "0 10 * * 1-5" }; + const recovered = { ...schedule, cron: "0 11 * * 1-5" }; + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(reconciliation.promise) + .mockResolvedValue([failed]); + vi.mocked(updateLocalScheduleCron) + .mockRejectedValueOnce(new Error("Timer replacement failed")) + .mockResolvedValueOnce(recovered); + const client = createQueryClient(); + const view = renderPanel(client); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + let input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: failed.cron } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + expect( + await screen.findByText("Timer replacement failed"), + ).toBeInTheDocument(); + expect(input).toHaveAttribute("aria-invalid", "true"); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("Timer replacement failed"), + ); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + await act(async () => { + reconciliation.resolve([failed]); + await reconciliation.promise; + }); + await waitFor(() => + expect( + screen.queryByText("Checking the schedule's current state…"), + ).not.toBeInTheDocument(), + ); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { + name: "Recover schedule for weekday-report", + }), + ).toBeInTheDocument(); + view.unmount(); + renderPanel(client); + expect( + await screen.findByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + await waitFor(() => + expect(screen.getByRole("button", { name: "Refresh" })).toBeEnabled(), + ); + const readsBeforeRefresh = vi.mocked(listLocalSchedules).mock.calls.length; + fireEvent.click(screen.getByRole("button", { name: "Refresh" })); + await waitFor(() => + expect(listLocalSchedules).toHaveBeenCalledTimes(readsBeforeRefresh + 1), + ); + fireEvent.click( + screen.getByRole("button", { + name: "Recover schedule for weekday-report", + }), + ); + input = screen.getByLabelText("Cron expression for weekday-report"); + expect(input).toHaveValue(failed.cron); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("This same expression already failed"), + ); + fireEvent.change(input, { target: { value: recovered.cron } }); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + vi.mocked(listLocalSchedules).mockResolvedValue([recovered]); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect( + screen.queryByLabelText("Cron expression for weekday-report"), + ).not.toBeInTheDocument(), + ); + expect(updateLocalScheduleCron).toHaveBeenNthCalledWith( + 2, + schedule.id, + recovered.cron, + ); + expect( + screen.queryByText("Schedule timer needs verification"), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ).toHaveFocus(); + }); + + it("surfaces a backend rejection and retains the user's draft", async () => { + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockResolvedValueOnce([{ ...schedule, cron: "0 10 * * 1-5" }]); + vi.mocked(updateLocalScheduleCron).mockRejectedValue({ + message: "Update failed", + data: "schedule rejected", + }); + renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + + expect(await screen.findByText("schedule rejected")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + const retryInput = screen.getByLabelText( + "Cron expression for weekday-report", + ); + expect(retryInput).toHaveValue("0 10 * * 1-5"); + + fireEvent.change(retryInput, { target: { value: "0 11 * * 1-5" } }); + fireEvent.change(retryInput, { target: { value: "0 10 * * 1-5" } }); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + expect(updateLocalScheduleCron).toHaveBeenCalledOnce(); + }); + + it("waits for reconciliation before saving an alternate after rejection", async () => { + vi.mocked(updateLocalScheduleCron) + .mockRejectedValueOnce({ + message: "Update failed", + data: "schedule rejected", + }) + .mockResolvedValueOnce({ ...schedule, cron: "0 11 * * 1-5" }); + const reconciliation = deferred<(typeof schedule)[]>(); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(reconciliation.promise) + .mockResolvedValueOnce([{ ...schedule, cron: "0 11 * * 1-5" }]); + renderPanel(); + + await screen.findByText("weekday-report"); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + let input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + expect(await screen.findByText("schedule rejected")).toBeInTheDocument(); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: "0 11 * * 1-5" } }, + ); + await waitFor(() => + expect( + screen.getByRole("button", { name: /Save|Saving/ }), + ).toBeDisabled(), + ); + expect( + screen.getByText("Checking the schedule's current state…"), + ).toBeInTheDocument(); + expect(updateLocalScheduleCron).toHaveBeenCalledOnce(); + await act(async () => { + reconciliation.resolve([{ ...schedule, cron: "0 10 * * 1-5" }]); + await reconciliation.promise; + }); + await waitFor(() => + expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(), + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect(updateLocalScheduleCron).toHaveBeenCalledTimes(2), + ); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(3)); + await waitFor(() => + expect(screen.getByText("0 11 * * 1-5")).toBeInTheDocument(), + ); + + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: "0 10 * * 1-5" } }); + await waitFor(() => + expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(), + ); + }); + + it("blocks overlapping updates after navigation while the previous save is pending", async () => { + const pending = deferred(); + vi.mocked(updateLocalScheduleCron).mockReturnValueOnce(pending.promise); + const client = createQueryClient(); + const view = renderPanel(client); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: "0 10 * * 1-5" } }, + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(updateLocalScheduleCron).toHaveBeenCalledOnce()); + view.unmount(); + renderPanel(client); + const edit = await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }); + expect(edit).toBeDisabled(); + fireEvent.click(edit); + expect( + screen.queryByLabelText("Cron expression for weekday-report"), + ).not.toBeInTheDocument(); + expect(updateLocalScheduleCron).toHaveBeenCalledOnce(); + await act(async () => { + pending.resolve({ ...schedule, cron: "0 10 * * 1-5" }); + await pending.promise; + }); + await waitFor(() => expect(edit).toBeEnabled()); + }); + + it("does not let another row's older failure reconciliation overwrite a successful edit", async () => { + const other = { ...schedule, id: "other-report" }; + const recoveredOther = { ...other, cron: "0 11 * * 1-5" }; + const reconciliation = deferred<(typeof schedule)[]>(); + let staleResponseDelivered = false; + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule, other]) + .mockReturnValueOnce( + reconciliation.promise.then((jobs) => { + staleResponseDelivered = true; + return jobs; + }), + ) + .mockResolvedValue([schedule, recoveredOther]); + vi.mocked(updateLocalScheduleCron) + .mockRejectedValueOnce(new Error("first update failed")) + .mockResolvedValueOnce(recoveredOther); + renderPanel(); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: "0 10 * * 1-5" } }, + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await screen.findByText("first update failed"); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + fireEvent.click( + screen.getByRole("button", { + name: "Edit cron schedule for other-report", + }), + ); + fireEvent.change( + screen.getByLabelText("Cron expression for other-report"), + { target: { value: recoveredOther.cron } }, + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect(updateLocalScheduleCron).toHaveBeenNthCalledWith( + 2, + other.id, + recoveredOther.cron, + ), + ); + await waitFor(() => + expect( + screen.queryByLabelText("Cron expression for other-report"), + ).not.toBeInTheDocument(), + ); + expect(screen.getByText(recoveredOther.cron)).toBeInTheDocument(); + await act(async () => { + reconciliation.resolve([schedule, other]); + await reconciliation.promise; + }); + expect(staleResponseDelivered).toBe(true); + expect(screen.getByText(recoveredOther.cron)).toBeInTheDocument(); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + }); + it("shows the scheduler error when the first local-schedule request fails", async () => { vi.mocked(listLocalSchedules).mockRejectedValue( new Error("scheduler disabled"), diff --git a/src/features/automations/ui/LocalSchedulesPanel.tsx b/src/features/automations/ui/LocalSchedulesPanel.tsx index 4099333c8..052cc8348 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.tsx @@ -7,6 +7,7 @@ import { LOCAL_SCHEDULES_REFETCH_INTERVAL_MS, listLocalSchedules, } from "@/features/automations/api/localSchedules"; +import { LocalScheduleCronEditor } from "@/features/automations/ui/LocalScheduleCronEditor"; import { EmptyState } from "@/features/automations/ui/RunOutput"; import { useLocaleFormatting } from "@/shared/i18n"; import { Alert, AlertTitle, AlertDescription } from "@/shared/ui/alert"; @@ -135,6 +136,7 @@ export function LocalSchedulesPanel() { ) : null} + ))} diff --git a/src/shared/i18n/locales/en/automations.json b/src/shared/i18n/locales/en/automations.json index 0266b5b30..662da0c68 100644 --- a/src/shared/i18n/locales/en/automations.json +++ b/src/shared/i18n/locales/en/automations.json @@ -48,7 +48,25 @@ "unknownLastRun": "Unknown", "paused": "Paused", "running": "Running", - "lastRun": "Last run: {{time}}" + "lastRun": "Last run: {{time}}", + "editCron": "Edit cron schedule for {{id}}", + "uncertainTitle": "Schedule timer needs verification", + "recoverCron": "Recover schedule for {{id}}", + "recoverAction": "Recover schedule", + "cronLabel": "Cron expression for {{id}}", + "cronHelp": "Use five fields (minute hour day-of-month month weekday) or six fields with seconds first. Goose uses this computer's time zone; day-of-month and weekday must both match when both are specified. This editor supports numeric wildcards, lists, ranges, steps, and weekday names. Custom edits must recur monthly or more often. Use a wildcard for month and for either day-of-month or weekday. Goose may remove the old timer if an unexpected update fails; verify the schedule after a save error.", + "saving": "Saving…", + "reconcilingSchedule": "Checking the schedule's current state…", + "updateError": "Could not update the Goose schedule.", + "updateFailureCaution": "The scheduler may have removed the old timer if this update failed unexpectedly. Verify the schedule in Goose before relying on it.", + "refreshAfterUpdateError": "The refresh also failed, so the schedule's current state is unknown.", + "failedUpdateRetry": "This same expression already failed. Do not retry it: Goose may have removed the old timer. Verify the schedule or try a different valid expression.", + "changedElsewhere": "This schedule changed elsewhere while you were editing. Cancel and reopen to review the latest value.", + "validation": { + "fieldCount": "Enter a five- or six-field cron expression.", + "unsupported": "This expression is invalid or uses syntax this editor cannot safely update. The existing schedule was not changed.", + "safeWindow": "Use a wildcard for month and a wildcard in either day-of-month or weekday. Goose requires both day fields to match when neither is a wildcard, and the scheduler combines them with AND." + } }, "builder": { "blockedBody": "The last response wasn't a valid automation. Try rephrasing what you want.", diff --git a/src/shared/i18n/locales/es/automations.json b/src/shared/i18n/locales/es/automations.json index 9bf9d4243..e6a6df0dd 100644 --- a/src/shared/i18n/locales/es/automations.json +++ b/src/shared/i18n/locales/es/automations.json @@ -40,15 +40,33 @@ "hubLocalOnlyDescription": "Consulta las programaciones configuradas en tu instancia local de Goose.", "loading": "Cargando programaciones de Goose", "loadErrorTitle": "No se pudieron cargar las programaciones de Goose", - "loadErrorDescription": "El programador local no esta disponible o Goose no esta conectado.", - "refreshError": "No se pudieron actualizar las programaciones: {{error}}. Se muestra la ultima lista cargada.", - "emptyTitle": "Aun no hay programaciones locales", - "emptyBody": "Las programaciones que crees en Goose apareceran aqui. Usa la CLI de Goose u otro cliente de Goose para programar y configurar recetas.", + "loadErrorDescription": "El programador local no está disponible o Goose no está conectado.", + "refreshError": "No se pudieron actualizar las programaciones: {{error}}. Se muestra la última lista cargada.", + "emptyTitle": "Aún no hay programaciones locales", + "emptyBody": "Las programaciones que crees en Goose aparecerán aquí. Usa la CLI de Goose u otro cliente de Goose para programar y configurar recetas.", "active": "Activa", "unknownLastRun": "Desconocida", "paused": "En pausa", - "running": "En ejecucion", - "lastRun": "Ultima ejecucion: {{time}}" + "running": "En ejecución", + "lastRun": "Última ejecución: {{time}}", + "editCron": "Editar la expresión cron de {{id}}", + "uncertainTitle": "Es necesario verificar el temporizador", + "recoverCron": "Recuperar la programación de {{id}}", + "recoverAction": "Recuperar programación", + "cronLabel": "Expresión cron de {{id}}", + "cronHelp": "Usa cinco campos (minuto hora día-del-mes mes día-de-la-semana) o seis campos con segundos al inicio. Goose usa la zona horaria de este equipo; si especificas día-del-mes y día-de-la-semana, ambos deben coincidir. Este editor admite comodines numéricos, listas, rangos, pasos y nombres de días. Las ediciones personalizadas deben repetirse cada mes o más frecuentemente. Usa un comodín para el mes y para día-del-mes o día-de-la-semana. Goose podría quitar el temporizador anterior si una actualización falla inesperadamente; verifica la programación después de un error al guardar.", + "saving": "Guardando…", + "reconcilingSchedule": "Comprobando el estado actual de la programación…", + "updateError": "No se pudo actualizar la programación de Goose.", + "updateFailureCaution": "Es posible que el programador haya quitado el temporizador anterior si la actualización falló inesperadamente. Verifica la programación en Goose antes de depender de ella.", + "refreshAfterUpdateError": "La actualización de la lista también falló, así que se desconoce el estado actual de la programación.", + "failedUpdateRetry": "Esta misma expresión ya falló. No la reintentes: es posible que Goose haya quitado el temporizador anterior. Verifica la programación o prueba otra expresión válida.", + "changedElsewhere": "Esta programación cambió mientras la editabas. Cancela y vuelve a abrirla para revisar el valor más reciente.", + "validation": { + "fieldCount": "Ingresa una expresión cron de cinco o seis campos.", + "unsupported": "La expresión no es válida o usa una sintaxis que este editor no puede actualizar de forma segura. La programación existente no cambió.", + "safeWindow": "Usa un comodín para el mes y un comodín en día-del-mes o día-de-la-semana. Goose exige que ambos campos de día coincidan si ninguno es comodín y los combina con AND." + } }, "builder": { "blockedBody": "La última respuesta no fue una automatización válida. Intenta reformular lo que quieres.",