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..161cfa700 100644 --- a/src/features/automations/api/localSchedules.test.ts +++ b/src/features/automations/api/localSchedules.test.ts @@ -1,18 +1,35 @@ 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", () => ({ + captureBackendConnectionGeneration: vi.fn(() => ({ + isCurrent: () => true, + invalidate: vi.fn().mockResolvedValue(undefined), + })), 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 +56,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..e4f2e09d1 100644 --- a/src/features/automations/api/localSchedules.ts +++ b/src/features/automations/api/localSchedules.ts @@ -1,11 +1,87 @@ import type { ScheduledJobDto } from "@aaif/goose-sdk"; -import { getClient } from "@/shared/api/acpConnection"; +import { LocalScheduleRequestError } from "@/features/automations/lib/localScheduleErrors"; +import { + captureBackendConnectionGeneration, + getClient, +} from "@/shared/api/acpConnection"; +import { LOCAL_BACKEND_ID } from "@/shared/api/acpBackendId"; export const LOCAL_SCHEDULES_QUERY_KEY = ["local-goose-schedules"] as const; export const LOCAL_SCHEDULES_REFETCH_INTERVAL_MS = 15_000; +export const LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS = 60_000; +export const LOCAL_SCHEDULE_READ_TIMEOUT_MS = 15_000; + +/** Follow the session mutation fence: abandon local work before retiring transport. */ +async function runScheduleRequest( + kind: "update" | "list", + request: (client: Awaited>) => Promise, +): Promise { + // Capture before initialization too: a delayed client must not send on a + // replacement after this operation's deadline has already expired. + const generation = captureBackendConnectionGeneration(LOCAL_BACKEND_ID); + let abandoned = false; + let timer: ReturnType | undefined; + const assertActive = () => { + if (abandoned || !generation.isCurrent()) { + throw new LocalScheduleRequestError("connectionChanged"); + } + }; + try { + return await Promise.race([ + (async () => { + const client = await getClient(); + assertActive(); + const result = await request(client); + assertActive(); + return result; + })(), + new Promise((_, reject) => { + timer = setTimeout( + () => { + abandoned = true; + if (kind === "update") { + // Detachment is synchronous; cleanup may never settle. The captured + // generation cannot detach a newer replacement. Never await cleanup. + void generation.invalidate().catch((error) => { + console.error( + "Failed to retire timed-out schedule transport:", + error, + ); + }); + } + reject( + new LocalScheduleRequestError( + kind === "update" ? "updateTimeout" : "listTimeout", + ), + ); + }, + kind === "update" + ? LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS + : LOCAL_SCHEDULE_READ_TIMEOUT_MS, + ); + }), + ]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } +} export async function listLocalSchedules(): Promise { - const client = await getClient(); - const response = await client.goose.GooseUnstableSchedulesList({}); - return response.jobs; + return runScheduleRequest("list", async (client) => { + const response = await client.goose.GooseUnstableSchedulesList({}); + return response.jobs; + }); +} + +export async function updateLocalScheduleCron( + scheduleId: string, + cron: string, +): Promise { + return runScheduleRequest("update", async (client) => { + 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..9f83ffa9d --- /dev/null +++ b/src/features/automations/lib/localScheduleCron.test.ts @@ -0,0 +1,58 @@ +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 * * MON-FRI", + "0 9 * * MON,WED,FRI", + "0 9 * * MON-FRI/2", + " 0 9 * * 1-5 ", + ])("accepts supported cron expression %s", (cron) => { + expect(validateLocalScheduleCron(cron)).toBeNull(); + }); + + it.each([ + "", + "@daily", + "0 9 * * JAN", + "0 MON * * *", + "JAN 9 * * *", + "0 0 9 * * DEC", + "MON 0 9 * * *", + "0 0 MON * * *", + "0 0 9 MON * *", + "0 9 * * */MON", + "0 9 * * MON-FRI/TUE", + "0 9 * * MON/WED", + "0 9 * * JAN-MAR", + "0 9 * * 1/JAN", + "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..83d68692f --- /dev/null +++ b/src/features/automations/lib/localScheduleCron.ts @@ -0,0 +1,142 @@ +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, +}; + +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) { + if (!/^\d+$/.test(stepText)) return false; + const step = Number(stepText); + if ( + !Number.isSafeInteger(step) || + 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"; + } + + // Names are interpreted only by their owning field, never across the + // expression; validate the same text that will be sent to Goose. + const fieldsWithSeconds = fields.length === 5 ? ["0", ...fields] : fields; + + 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/localScheduleErrors.ts b/src/features/automations/lib/localScheduleErrors.ts new file mode 100644 index 000000000..cef835435 --- /dev/null +++ b/src/features/automations/lib/localScheduleErrors.ts @@ -0,0 +1,8 @@ +type RequestErrorCode = "updateTimeout" | "listTimeout" | "connectionChanged"; + +export class LocalScheduleRequestError extends Error { + constructor(readonly code: RequestErrorCode) { + super(code); + this.name = "LocalScheduleRequestError"; + } +} diff --git a/src/features/automations/lib/localScheduleSpanish.test.ts b/src/features/automations/lib/localScheduleSpanish.test.ts new file mode 100644 index 000000000..15fcc2a2a --- /dev/null +++ b/src/features/automations/lib/localScheduleSpanish.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import esAutomations from "@/shared/i18n/locales/es/automations.json"; + +describe("Spanish local schedule copy", () => { + it.each([ + [ + "loadErrorDescription", + "Puede que el programador local no esté disponible o que 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."], + [ + "updateTimeout", + "La actualización de la programación agotó el tiempo de espera. Se cerró su conexión y se desconoce el estado del temporizador. Verifica la programación en Goose antes de recuperarla con otra expresión.", + ], + [ + "listTimeout", + "La actualización de la lista agotó el tiempo de espera. Actualiza para reintentar; ver una programación en la lista no verifica su temporizador.", + ], + [ + "connectionChanged", + "La conexión de Goose cambió antes de confirmar la solicitud de programación. Actualiza y verifica la programación antes de reintentar.", + ], + [ + "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..a4e6f0cd2 --- /dev/null +++ b/src/features/automations/ui/LocalScheduleCronEditor.tsx @@ -0,0 +1,326 @@ +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { + isCancelledError, + 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 { LocalScheduleRequestError } from "@/features/automations/lib/localScheduleErrors"; +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 [failedSubmittedCron, setFailedSubmittedCron] = useState( + null, + ); + 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: () => { + // A confirmed update establishes the new timer. Readback is independently + // bounded in the API; its query status is visible in LocalSchedulesPanel. + // Never retain the per-schedule mutation lock waiting for a list response. + void 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) => { + setFailedSubmittedCron(variables.cron); + setError( + mutationError instanceof LocalScheduleRequestError + ? t(`localSchedules.${mutationError.code}`) + : 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; + + if (mutationError instanceof LocalScheduleRequestError) { + // This generation is retired or replaced. Release the shared save lock + // now; even a stalled readback must not extend the mutation deadline. + setIsReconciling(false); + void queryClient.invalidateQueries({ + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + }); + return; + } + + 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 (refreshError) { + if (isCancelledError(refreshError)) return; + setError((current) => + current + ? `${current} ${t("localSchedules.refreshAfterUpdateError")}` + : t("localSchedules.refreshAfterUpdateError"), + ); + } finally { + setIsReconciling(false); + } + }, + }); + + const scheduleChangedElsewhere = + isEditing && + !isSaving && + !isReconciling && + schedule.cron !== baselineCron && + // The rejected update may have persisted its submitted cron before failing. + // Only that expected value can waive the original baseline mismatch. + schedule.cron !== failedSubmittedCron; + + const displayedError = scheduleChangedElsewhere + ? t("localSchedules.changedElsewhere") + : error && errorCoversCurrentDraft + ? error + : 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); + setFailedSubmittedCron(null); + 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/LocalScheduleLiveness.test.tsx b/src/features/automations/ui/LocalScheduleLiveness.test.tsx new file mode 100644 index 000000000..97ac15c20 --- /dev/null +++ b/src/features/automations/ui/LocalScheduleLiveness.test.tsx @@ -0,0 +1,365 @@ +import { + QueryClient, + QueryClientProvider, + notifyManager, +} from "@tanstack/react-query"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + LOCAL_SCHEDULE_READ_TIMEOUT_MS, + LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS, + updateLocalScheduleCron, +} from "@/features/automations/api/localSchedules"; +import { getBackendConnection } from "@/shared/api/acpConnection"; +import { LocalSchedulesPanel } from "@/features/automations/ui/LocalSchedulesPanel"; + +const mocks = vi.hoisted(() => ({ + update: vi.fn(), + list: vi.fn(), + stream: vi.fn(), + invoke: vi.fn(), +})); +vi.mock("@tauri-apps/api/core", () => ({ + invoke: (...args: unknown[]) => mocks.invoke(...args), +})); +vi.mock("@/shared/api/createWebSocketStream", () => ({ + createWebSocketStream: () => mocks.stream(), +})); +vi.mock("@agentclientprotocol/sdk", () => ({ PROTOCOL_VERSION: 1 })); +vi.mock("@aaif/goose-sdk", () => ({ + DEFAULT_GOOSE_MCP_HOST_CAPABILITIES: {}, + GooseClient: class { + initialize = vi.fn().mockResolvedValue(undefined); + closed = new Promise(() => {}); + goose = { + GooseUnstableSchedulesList: mocks.list, + GooseUnstableSchedulesUpdate: mocks.update, + }; + }, +})); + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} +const schedule = { + id: "weekday-report", + source: "/tmp/weekday-report.yaml", + cron: "0 9 * * 1-5", + paused: false, + currentlyRunning: false, +}; +const failed = { ...schedule, cron: "0 10 * * 1-5" }; +const recovered = { ...schedule, cron: "0 11 * * 1-5" }; +let client: QueryClient; +let views: Array>; +function mount() { + const view = render( + + + , + ); + views.push(view); + return view; +} +async function tick(ms = 0) { + await act(async () => { + await vi.advanceTimersByTimeAsync(ms); + }); +} +function edit(cron: string, recovery = false) { + fireEvent.click( + screen.getByRole("button", { + name: `${recovery ? "Recover schedule" : "Edit cron schedule"} for weekday-report`, + }), + ); + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: cron } }, + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); +} + +beforeEach(async () => { + vi.useFakeTimers(); + notifyManager.setScheduler((callback) => callback()); + mocks.update.mockReset(); + mocks.list.mockReset(); + mocks.stream.mockReset(); + mocks.invoke.mockReset(); + void getBackendConnection("local").invalidate(); + mocks.invoke.mockResolvedValue("ws://local"); + mocks.stream.mockImplementation(() => ({ + writable: { abort: vi.fn().mockResolvedValue(undefined) }, + })); + mocks.list.mockResolvedValue({ jobs: [schedule] }); + client = new QueryClient({ + defaultOptions: { queries: { retry: false, refetchOnMount: false } }, + }); + views = []; +}); +afterEach(async () => { + views.forEach((view) => { + view.unmount(); + }); + client.clear(); + void getBackendConnection("local").invalidate(); + notifyManager.setScheduler((callback) => setTimeout(callback, 0)); + vi.useRealTimers(); +}); + +describe("schedule request liveness through the real connection generation", () => { + it.each([ + "success", + "error", + ] as const)("releases a remounted lock after timeout and fences late %s during fresh recovery", async (late) => { + const old = deferred<{ job: typeof schedule }>(); + const replacement = deferred<{ job: typeof schedule }>(); + mocks.update + .mockReturnValueOnce(old.promise) + .mockReturnValueOnce(replacement.promise); + const firstView = mount(); + await tick(); + edit(failed.cron); + await tick(); + expect(mocks.update).toHaveBeenCalledExactlyOnceWith({ + scheduleId: schedule.id, + cron: failed.cron, + }); + const connection = getBackendConnection("local"); + const oldClient = connection.getClientSync(); + mocks.list.mockReturnValueOnce(new Promise(() => {})); + const oldStream = mocks.stream.mock.results[0].value; + oldStream.writable.abort.mockReturnValue(new Promise(() => {})); + firstView.unmount(); + mount(); + await tick(); + expect( + screen.getByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ).toBeDisabled(); + expect(client.isMutating()).toBe(1); + await tick(LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS); + expect(client.isMutating()).toBe(0); + expect(oldStream.writable.abort).toHaveBeenCalledOnce(); + expect(connection.getClientSync()).not.toBe(oldClient); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { + name: "Recover schedule for weekday-report", + }), + ).toBeEnabled(); + edit(recovered.cron, true); + await tick(); + expect(mocks.update).toHaveBeenCalledTimes(2); + expect(mocks.stream).toHaveBeenCalledTimes(2); + const newClient = connection.getClientSync(); + expect(client.isMutating()).toBe(1); + replacement.reject(new Error("new rejection")); + await tick(); + expect(client.isMutating()).toBe(0); + expect( + client.getQueryData(["local-goose-schedule-failures", schedule.id]), + ).toEqual([failed.cron, recovered.cron]); + await act(async () => { + if (late === "success") old.resolve({ job: failed }); + else old.reject(new Error("old rejection")); + await old.promise.catch(() => {}); + }); + await tick(); + expect(client.isMutating()).toBe(0); + expect( + client.getQueryData>([ + "local-goose-schedules", + ])?.[0].cron, + ).toBe(schedule.cron); + expect( + client.getQueryData(["local-goose-schedule-failures", schedule.id]), + ).toEqual([failed.cron, recovered.cron]); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + expect(connection.getClientSync()).toBe(newClient); + }); + + it("releases the open editor at its deadline with an accessible uncertainty error", async () => { + mocks.update.mockReturnValue(new Promise(() => {})); + mount(); + await tick(); + edit(failed.cron); + await tick(); + expect(mocks.update).toHaveBeenCalledOnce(); + expect( + screen.getByLabelText("Cron expression for weekday-report"), + ).toBeDisabled(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); + await tick(LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS); + const input = screen.getByLabelText("Cron expression for weekday-report"); + expect(client.isMutating()).toBe(0); + expect(input).toBeEnabled(); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("The schedule update timed out"), + ); + expect(screen.getByRole("button", { name: "Cancel" })).toBeEnabled(); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect( + screen.getByRole("button", { + name: "Recover schedule for weekday-report", + }), + ).toBeEnabled(); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + }); + + it("does not let an older timeout detach a newer replacement transport", async () => { + mocks.update.mockReturnValue(new Promise(() => {})); + const first = updateLocalScheduleCron("first", failed.cron); + const firstRejected = expect(first).rejects.toMatchObject({ + code: "updateTimeout", + }); + await tick(10_000); + const second = updateLocalScheduleCron("second", recovered.cron); + const secondRejected = expect(second).rejects.toMatchObject({ + code: "updateTimeout", + }); + await tick(); + expect(mocks.update).toHaveBeenCalledTimes(2); + await tick(50_000); + await firstRejected; + const connection = getBackendConnection("local"); + expect(connection.getClientSync()).toBeNull(); + const fresh = await connection.getClient(); + const freshStream = mocks.stream.mock.results[1].value; + await tick(10_000); + await secondRejected; + expect(connection.getClientSync()).toBe(fresh); + expect(freshStream.writable.abort).not.toHaveBeenCalled(); + }); + + it("does not send an update if client acquisition finishes after its deadline", async () => { + const endpoint = deferred(); + mocks.invoke.mockReturnValueOnce(endpoint.promise); + const pending = updateLocalScheduleCron(schedule.id, failed.cron); + const rejected = expect(pending).rejects.toMatchObject({ + code: "updateTimeout", + }); + expect(mocks.invoke).toHaveBeenCalledOnce(); + await tick(LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS); + await rejected; + endpoint.resolve("ws://old"); + await tick(); + expect(mocks.update).not.toHaveBeenCalled(); + await getBackendConnection("local").getClient(); + expect(mocks.stream).toHaveBeenCalledOnce(); + }); + + it("finishes confirmed success before a stalled readback and makes refresh recoverable", async () => { + const readback = deferred<{ jobs: (typeof schedule)[] }>(); + mocks.update.mockResolvedValue({ job: recovered }); + mocks.list + .mockResolvedValueOnce({ jobs: [schedule] }) + .mockReturnValueOnce(readback.promise) + .mockResolvedValue({ jobs: [recovered] }); + mount(); + await tick(); + edit(recovered.cron); + await tick(); + expect(mocks.update).toHaveBeenCalledOnce(); + expect(mocks.list).toHaveBeenCalledTimes(2); + expect(client.isMutating()).toBe(0); + expect( + screen.queryByLabelText("Cron expression for weekday-report"), + ).not.toBeInTheDocument(); + expect( + screen.queryByText("Schedule timer needs verification"), + ).not.toBeInTheDocument(); + await tick(LOCAL_SCHEDULE_READ_TIMEOUT_MS); + expect(screen.getByText(/Schedule refresh timed out/)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Refresh" })).toBeEnabled(); + fireEvent.click(screen.getByRole("button", { name: "Refresh" })); + await tick(); + expect(mocks.list).toHaveBeenCalledTimes(3); + expect(screen.getByText(recovered.cron)).toBeInTheDocument(); + readback.resolve({ jobs: [failed] }); + await tick(); + expect(screen.getByText(recovered.cron)).toBeInTheDocument(); + expect( + screen.queryByText(/Schedule refresh timed out/), + ).not.toBeInTheDocument(); + }); + + it("bounds stalled failure reconciliation without losing uncertainty", async () => { + mocks.update.mockRejectedValue(new Error("replacement rejected")); + mocks.list + .mockResolvedValueOnce({ jobs: [schedule] }) + .mockReturnValueOnce(new Promise(() => {})); + mount(); + await tick(); + edit(failed.cron); + await tick(); + expect(mocks.update).toHaveBeenCalledOnce(); + expect(mocks.list).toHaveBeenCalledTimes(2); + expect(client.isMutating()).toBe(1); + await tick(LOCAL_SCHEDULE_READ_TIMEOUT_MS); + expect(client.isMutating()).toBe(0); + expect(screen.getByText(/The refresh also failed/)).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + }); + + it("handles rejection before the deadline and ordinary success without retiring transport", async () => { + mocks.update + .mockRejectedValueOnce(new Error("replacement rejected")) + .mockResolvedValueOnce({ job: recovered }); + mount(); + await tick(); + edit(failed.cron); + await tick(); + expect(screen.getByText("replacement rejected")).toBeInTheDocument(); + expect(client.isMutating()).toBe(0); + const stream = mocks.stream.mock.results[0].value; + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: recovered.cron } }, + ); + mocks.list.mockResolvedValue({ jobs: [recovered] }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await tick(); + expect(mocks.update).toHaveBeenCalledTimes(2); + expect(client.isMutating()).toBe(0); + expect( + screen.queryByText("Schedule timer needs verification"), + ).not.toBeInTheDocument(); + await tick(LOCAL_SCHEDULE_UPDATE_TIMEOUT_MS); + expect(stream.writable.abort).not.toHaveBeenCalled(); + }); + + it("rejects a response from a generation replaced before its deadline", async () => { + const response = deferred<{ job: typeof schedule }>(); + mocks.update.mockReturnValue(response.promise); + const pending = updateLocalScheduleCron(schedule.id, failed.cron); + const rejected = expect(pending).rejects.toMatchObject({ + code: "connectionChanged", + }); + await tick(); + expect(mocks.update).toHaveBeenCalledOnce(); + void getBackendConnection("local").invalidate(); + await getBackendConnection("local").getClient(); + response.resolve({ job: failed }); + await rejected; + }); +}); diff --git a/src/features/automations/ui/LocalSchedulesPanel.test.tsx b/src/features/automations/ui/LocalSchedulesPanel.test.tsx index 1d07f0202..6d89a1b4a 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.test.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.test.tsx @@ -1,20 +1,44 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen } from "@testing-library/react"; +import { + act, + fireEvent, + render, + screen, + waitFor, + within, +} 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 +56,7 @@ const schedule = { describe("LocalSchedulesPanel", () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); vi.mocked(listLocalSchedules).mockResolvedValue([schedule]); }); @@ -90,6 +114,624 @@ 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 an unrelated external cron after a rejected update until reopening", async () => { + const reconciliation = deferred<(typeof schedule)[]>(); + const failedCron = "0 10 * * 1-5"; + const externalCron = "0 12 * * 1-5"; + const recoveryCron = "0 11 * * 1-5"; + vi.mocked(updateLocalScheduleCron).mockRejectedValueOnce( + new Error("replacement rejected"), + ); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(reconciliation.promise); + renderPanel(); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: failedCron } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await screen.findByText("replacement rejected"); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + await act(async () => { + reconciliation.resolve([{ ...schedule, cron: externalCron }]); + await reconciliation.promise; + }); + fireEvent.change(input, { target: { value: recoveryCron } }); + await waitFor(() => + expect(input).toHaveAccessibleDescription( + expect.stringContaining("This schedule changed elsewhere"), + ), + ); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + expect(updateLocalScheduleCron).toHaveBeenCalledExactlyOnceWith( + schedule.id, + failedCron, + ); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + fireEvent.click( + screen.getByRole("button", { + name: "Recover schedule for weekday-report", + }), + ); + expect( + screen.getByLabelText("Cron expression for weekday-report"), + ).toHaveValue(externalCron); + expect( + screen.queryByText(/^This schedule changed elsewhere/), + ).not.toBeInTheDocument(); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + }); + + it.each([ + "0 9 * * JAN", + "0 MON * * *", + "0 9 * * */MON", + ])("blocks unsafe alias input %s before the scheduler RPC", async (cron) => { + renderPanel(); + fireEvent.click( + await screen.findByRole("button", { + name: "Edit cron schedule for weekday-report", + }), + ); + const input = screen.getByLabelText("Cron expression for weekday-report"); + fireEvent.change(input, { target: { value: cron } }); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("This expression is invalid or uses syntax"), + ); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + expect(updateLocalScheduleCron).not.toHaveBeenCalled(); + }); + + it("does not report a refresh failure when cancellation rejects reconciliation", async () => { + const reconciliation = deferred<(typeof schedule)[]>(); + const client = createQueryClient(); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(reconciliation.promise); + vi.mocked(updateLocalScheduleCron).mockRejectedValueOnce( + new Error("replacement rejected"), + ); + 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 screen.findByText("replacement rejected"); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); + await act(async () => { + // Default revert cancellation can resolve with cached data. Explicitly + // exercise the rejecting cancellation path too. + await client.cancelQueries( + { queryKey: ["local-goose-schedules"] }, + { revert: false }, + ); + }); + await waitFor(() => expect(client.isMutating()).toBe(0)); + expect( + screen.queryByText(/The refresh also failed/), + ).not.toBeInTheDocument(); + expect(screen.getByText("replacement rejected")).toBeInTheDocument(); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + await act(async () => { + reconciliation.resolve([schedule]); + await reconciliation.promise; + }); + }); + + it("reports a genuine reconciliation failure without losing timer uncertainty", async () => { + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .mockRejectedValueOnce(new Error("list unavailable")); + vi.mocked(updateLocalScheduleCron).mockRejectedValueOnce( + new Error("replacement rejected"), + ); + 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" })); + expect( + await screen.findByText( + /The refresh also failed, so the schedule's current state is unknown/, + ), + ).toBeInTheDocument(); + expect(listLocalSchedules).toHaveBeenCalledTimes(2); + expect( + screen.getByText("Schedule timer needs verification"), + ).toBeInTheDocument(); + }); + + 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: "Edit cron schedule for other-report", + }), + ); + const otherInput = screen.getByLabelText( + "Cron expression for other-report", + ); + fireEvent.change(otherInput, { target: { value: recoveredOther.cron } }); + const otherArticle = otherInput.closest("article"); + if (!otherArticle) throw new Error("Missing other schedule row"); + fireEvent.click(within(otherArticle).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.queryByText(/The refresh also failed/), + ).not.toBeInTheDocument(); + expect( + screen.getByLabelText("Cron expression for weekday-report"), + ).toBeInTheDocument(); + 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..6e777d857 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.tsx @@ -7,6 +7,8 @@ import { LOCAL_SCHEDULES_REFETCH_INTERVAL_MS, listLocalSchedules, } from "@/features/automations/api/localSchedules"; +import { LocalScheduleCronEditor } from "@/features/automations/ui/LocalScheduleCronEditor"; +import { LocalScheduleRequestError } from "@/features/automations/lib/localScheduleErrors"; import { EmptyState } from "@/features/automations/ui/RunOutput"; import { useLocaleFormatting } from "@/shared/i18n"; import { Alert, AlertTitle, AlertDescription } from "@/shared/ui/alert"; @@ -38,8 +40,14 @@ export function LocalSchedulesPanel() { queryKey: LOCAL_SCHEDULES_QUERY_KEY, queryFn: listLocalSchedules, refetchInterval: LOCAL_SCHEDULES_REFETCH_INTERVAL_MS, + retry: false, }); const schedules = schedulesQuery.data ?? []; + const queryError = schedulesQuery.error; + const errorMessage = + queryError instanceof LocalScheduleRequestError + ? t(`localSchedules.${queryError.code}`) + : localErrorMessage(queryError, t("localSchedules.loadErrorDescription")); return (
@@ -74,22 +82,14 @@ export function LocalSchedulesPanel() { ) : ( <> {schedulesQuery.error ? (
{t("localSchedules.refreshError", { - error: localErrorMessage( - schedulesQuery.error, - t("localSchedules.loadErrorDescription"), - ), + error: errorMessage, })}
) : null} @@ -135,6 +135,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..95dce6c76 100644 --- a/src/shared/i18n/locales/en/automations.json +++ b/src/shared/i18n/locales/en/automations.json @@ -48,7 +48,28 @@ "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…", + "updateTimeout": "The schedule update timed out. Its connection was retired; the timer state is unknown. Verify the schedule in Goose before recovering with another expression.", + "listTimeout": "Schedule refresh timed out. Refresh to retry; listing a schedule does not verify its timer.", + "connectionChanged": "The Goose connection changed before the schedule request was confirmed. Refresh and verify the schedule before retrying.", + "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 223abd446..e267ad9c6 100644 --- a/src/shared/i18n/locales/es/automations.json +++ b/src/shared/i18n/locales/es/automations.json @@ -48,7 +48,28 @@ "unknownLastRun": "Desconocida", "paused": "En pausa", "running": "En ejecución", - "lastRun": "Última ejecución: {{time}}" + "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…", + "updateTimeout": "La actualización de la programación agotó el tiempo de espera. Se cerró su conexión y se desconoce el estado del temporizador. Verifica la programación en Goose antes de recuperarla con otra expresión.", + "listTimeout": "La actualización de la lista agotó el tiempo de espera. Actualiza para reintentar; ver una programación en la lista no verifica su temporizador.", + "connectionChanged": "La conexión de Goose cambió antes de confirmar la solicitud de programación. Actualiza y verifica la programación antes de reintentar.", + "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.",