From 9227e9fd83e16ae00678e22c7fe31f28ea36e9a8 Mon Sep 17 00:00:00 2001 From: Kyle De Freitas Date: Tue, 27 Oct 2026 15:15:33 -0400 Subject: [PATCH 1/4] feat(automations): configure local Goose schedules --- docs/plans/goose-schedules-in-berd.md | 4 +- .../automations/api/localSchedules.test.ts | 27 +- .../automations/api/localSchedules.ts | 12 + .../automations/lib/localScheduleCron.test.ts | 43 ++++ .../automations/lib/localScheduleCron.ts | 150 +++++++++++ .../ui/LocalScheduleCronEditor.tsx | 219 +++++++++++++++++ .../ui/LocalSchedulesPanel.test.tsx | 232 +++++++++++++++++- .../automations/ui/LocalSchedulesPanel.tsx | 2 + src/shared/i18n/locales/en/automations.json | 15 +- src/shared/i18n/locales/es/automations.json | 15 +- 10 files changed, 710 insertions(+), 9 deletions(-) create mode 100644 src/features/automations/lib/localScheduleCron.test.ts create mode 100644 src/features/automations/lib/localScheduleCron.ts create mode 100644 src/features/automations/ui/LocalScheduleCronEditor.tsx 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/ui/LocalScheduleCronEditor.tsx b/src/features/automations/ui/LocalScheduleCronEditor.tsx new file mode 100644 index 000000000..0ebca6a59 --- /dev/null +++ b/src/features/automations/ui/LocalScheduleCronEditor.tsx @@ -0,0 +1,219 @@ +import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useTranslation } from "react-i18next"; +import { IconCheck, IconPencil, IconX } from "@tabler/icons-react"; +import type { ScheduledJobDto } from "@aaif/goose-sdk"; +import { + LOCAL_SCHEDULES_QUERY_KEY, + updateLocalScheduleCron, +} from "@/features/automations/api/localSchedules"; +import { validateLocalScheduleCron } from "@/features/automations/lib/localScheduleCron"; +import { formatAcpErrorMessage } from "@/shared/api/acpErrors"; +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 editButtonRef = useRef(null); + const inputRef = useRef(null); + const wasEditingRef = useRef(false); + const restoreFocusAfterEditRef = useRef(false); + const [failedCrons, setFailedCrons] = useState>(() => new Set()); + 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.has(normalizedCron); + const updateMutation = useMutation({ + mutationFn: ({ scheduleId, cron }: { scheduleId: string; cron: string }) => + updateLocalScheduleCron(scheduleId, cron), + onMutate: async () => { + setIsSaving(true); + await queryClient.cancelQueries({ queryKey: LOCAL_SCHEDULES_QUERY_KEY }); + }, + onSuccess: async () => { + // Re-read the live scheduler after mutation instead of writing the + // response into cache: a newer poll may have completed while saving. + await queryClient.invalidateQueries({ + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + }); + setError(null); + setFailedCrons(new Set()); + setIsSaving(false); + restoreFocusAfterEditRef.current = + document.activeElement === inputRef.current; + setIsEditing(false); + }, + onError: (mutationError, variables) => { + setError( + formatAcpErrorMessage(mutationError, t("localSchedules.updateError")), + ); + setFailedCrons((previous) => new Set(previous).add(variables.cron)); + setIsSaving(false); + restoreFocusAfterEditRef.current = + document.activeElement === document.body || + document.activeElement === editButtonRef.current; + }, + }); + + const scheduleChangedElsewhere = + isEditing && !updateMutation.isError && schedule.cron !== baselineCron; + + 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 ( + !hasChanged || + (validationError && !hasKnownCron) || + sameAsFailedCron || + scheduleChangedElsewhere || + schedule.currentlyRunning + ) { + return; + } + setError(null); + updateMutation.mutate({ scheduleId: schedule.id, cron: normalizedCron }); + } + + if (!isEditing) { + return ( + + ); + } + + return ( +
+ + { + setDraft(event.target.value); + setError(null); + }} + aria-invalid={!!validationError || !!error} + aria-describedby={`local-schedule-cron-help-${schedule.id}`} + disabled={isSaving} + autoComplete="off" + spellCheck={false} + className="font-mono" + /> +

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

+ {scheduleChangedElsewhere ? ( +

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

+ ) : validationError && !hasKnownCron ? ( +

+ {t(`localSchedules.validation.${validationError}`)} +

+ ) : sameAsFailedCron && !error ? ( +

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

+ ) : error ? ( +
+

{error}

+

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

+
+ ) : null} +
+ + +
+
+ ); +} diff --git a/src/features/automations/ui/LocalSchedulesPanel.test.tsx b/src/features/automations/ui/LocalSchedulesPanel.test.tsx index 1d07f0202..2996b63aa 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.test.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.test.tsx @@ -1,16 +1,33 @@ 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 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 renderPanel() { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, @@ -32,7 +49,7 @@ const schedule = { describe("LocalSchedulesPanel", () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); vi.mocked(listLocalSchedules).mockResolvedValue([schedule]); }); @@ -90,6 +107,215 @@ 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(), + ); + 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(screen.getByRole("alert")).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("surfaces a backend rejection and retains the user's draft", async () => { + 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("allows an expression again after another update succeeds", async () => { + vi.mocked(updateLocalScheduleCron) + .mockRejectedValueOnce({ + message: "Update failed", + data: "schedule rejected", + }) + .mockResolvedValueOnce({ ...schedule, cron: "0 11 * * 1-5" }); + vi.mocked(listLocalSchedules) + .mockResolvedValueOnce([schedule]) + .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(); + + fireEvent.change( + screen.getByLabelText("Cron expression for weekday-report"), + { target: { value: "0 11 * * 1-5" } }, + ); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect(updateLocalScheduleCron).toHaveBeenCalledTimes(2), + ); + 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" } }); + expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(); + }); + 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..55dbad45b 100644 --- a/src/shared/i18n/locales/en/automations.json +++ b/src/shared/i18n/locales/en/automations.json @@ -48,7 +48,20 @@ "unknownLastRun": "Unknown", "paused": "Paused", "running": "Running", - "lastRun": "Last run: {{time}}" + "lastRun": "Last run: {{time}}", + "editCron": "Edit cron schedule for {{id}}", + "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…", + "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.", + "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 searches up to one year for the next run." + } }, "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..5827b7955 100644 --- a/src/shared/i18n/locales/es/automations.json +++ b/src/shared/i18n/locales/es/automations.json @@ -48,7 +48,20 @@ "unknownLastRun": "Desconocida", "paused": "En pausa", "running": "En ejecucion", - "lastRun": "Ultima ejecucion: {{time}}" + "lastRun": "Ultima ejecucion: {{time}}", + "editCron": "Editar la expresion cron de {{id}}", + "cronLabel": "Expresion cron de {{id}}", + "cronHelp": "Usa cinco campos (minuto hora dia-del-mes mes dia-de-la-semana) o seis campos con segundos al inicio. Goose usa la zona horaria de este equipo; si especificas dia-del-mes y dia-de-la-semana, ambos deben coincidir. Este editor admite comodines numericos, listas, rangos, pasos y nombres de dias. Las ediciones personalizadas deben repetirse cada mes o mas frecuentemente. Usa un comodin para el mes y para dia-del-mes o dia-de-la-semana. Goose podria quitar el temporizador anterior si una actualizacion falla inesperadamente; verifica la programacion despues de un error al guardar.", + "saving": "Guardando…", + "updateError": "No se pudo actualizar la programacion de Goose.", + "updateFailureCaution": "Es posible que el programador haya quitado el temporizador anterior si la actualizacion fallo inesperadamente. Verifica la programacion en Goose antes de depender de ella.", + "failedUpdateRetry": "Esta misma expresion ya fallo. No la reintentes: es posible que Goose haya quitado el temporizador anterior. Verifica la programacion o prueba otra expresion valida.", + "changedElsewhere": "Esta programacion cambio mientras la editabas. Cancela y vuelve a abrirla para revisar el valor mas reciente.", + "validation": { + "fieldCount": "Ingresa una expresion cron de cinco o seis campos.", + "unsupported": "La expresion no es valida o usa una sintaxis que este editor no puede actualizar de forma segura. La programacion existente no cambio.", + "safeWindow": "Usa un comodin para el mes y un comodin en dia-del-mes o dia-de-la-semana. Goose exige que ambos campos de dia coincidan si ninguno es comodin y busca la siguiente ejecucion hasta por un ano." + } }, "builder": { "blockedBody": "La última respuesta no fue una automatización válida. Intenta reformular lo que quieres.", From 43bd63eb7cc98d4c1fb90c6b45b2bcecb0d31b66 Mon Sep 17 00:00:00 2001 From: Kyle De Freitas Date: Tue, 27 Oct 2026 15:48:13 -0400 Subject: [PATCH 2/4] fix(automations): reconcile failed schedule updates --- .../ui/LocalScheduleCronEditor.tsx | 65 ++++++++++++++----- .../ui/LocalSchedulesPanel.test.tsx | 11 +++- src/shared/i18n/locales/en/automations.json | 3 +- src/shared/i18n/locales/es/automations.json | 3 +- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/features/automations/ui/LocalScheduleCronEditor.tsx b/src/features/automations/ui/LocalScheduleCronEditor.tsx index 0ebca6a59..47a514164 100644 --- a/src/features/automations/ui/LocalScheduleCronEditor.tsx +++ b/src/features/automations/ui/LocalScheduleCronEditor.tsx @@ -26,8 +26,11 @@ export function LocalScheduleCronEditor({ 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 shouldRestoreFocusOnSaveRef = useRef(false); const wasEditingRef = useRef(false); const restoreFocusAfterEditRef = useRef(false); const [failedCrons, setFailedCrons] = useState>(() => new Set()); @@ -41,6 +44,7 @@ export function LocalScheduleCronEditor({ updateLocalScheduleCron(scheduleId, cron), onMutate: async () => { setIsSaving(true); + setIsReconciling(false); await queryClient.cancelQueries({ queryKey: LOCAL_SCHEDULES_QUERY_KEY }); }, onSuccess: async () => { @@ -52,24 +56,49 @@ export function LocalScheduleCronEditor({ setError(null); setFailedCrons(new Set()); setIsSaving(false); + setIsReconciling(false); restoreFocusAfterEditRef.current = - document.activeElement === inputRef.current; + shouldRestoreFocusOnSaveRef.current || + editorRef.current?.contains(document.activeElement) === true; setIsEditing(false); }, - onError: (mutationError, variables) => { + onError: async (mutationError, variables) => { setError( formatAcpErrorMessage(mutationError, t("localSchedules.updateError")), ); setFailedCrons((previous) => new Set(previous).add(variables.cron)); setIsSaving(false); + setIsReconciling(true); restoreFocusAfterEditRef.current = - document.activeElement === document.body || - document.activeElement === editButtonRef.current; + shouldRestoreFocusOnSaveRef.current || + editorRef.current?.contains(document.activeElement) === true || + document.activeElement === document.body; + try { + await queryClient.refetchQueries( + { + queryKey: LOCAL_SCHEDULES_QUERY_KEY, + predicate: (query) => query.state.fetchStatus !== "paused", + }, + { throwOnError: true, cancelRefetch: false }, + ); + } catch { + setError((current) => + current + ? `${current} ${t("localSchedules.refreshAfterUpdateError")}` + : t("localSchedules.refreshAfterUpdateError"), + ); + } finally { + setIsReconciling(false); + } }, }); const scheduleChangedElsewhere = - isEditing && !updateMutation.isError && schedule.cron !== baselineCron; + isEditing && + !isReconciling && + !updateMutation.isError && + !error && + schedule.cron !== baselineCron; useLayoutEffect(() => { if (isEditing) { @@ -95,6 +124,7 @@ export function LocalScheduleCronEditor({ function beginEditing() { restoreFocusAfterEditRef.current = true; + shouldRestoreFocusOnSaveRef.current = false; setBaselineCron(schedule.cron); setDraft(schedule.cron); setError(null); @@ -102,6 +132,7 @@ export function LocalScheduleCronEditor({ } function cancelEditing() { + shouldRestoreFocusOnSaveRef.current = false; setBaselineCron(schedule.cron); setDraft(schedule.cron); setError(null); @@ -109,7 +140,10 @@ export function LocalScheduleCronEditor({ } function save() { + shouldRestoreFocusOnSaveRef.current = + editorRef.current?.contains(document.activeElement) === true; if ( + isReconciling || !hasChanged || (validationError && !hasKnownCron) || sameAsFailedCron || @@ -139,7 +173,7 @@ export function LocalScheduleCronEditor({ } return ( -
+
@@ -164,7 +198,14 @@ export function LocalScheduleCronEditor({ > {t("localSchedules.cronHelp")}

- {scheduleChangedElsewhere ? ( + {error ? ( +
+

{error}

+

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

+
+ ) : scheduleChangedElsewhere ? (

{t("localSchedules.changedElsewhere")}

@@ -172,17 +213,10 @@ export function LocalScheduleCronEditor({

{t(`localSchedules.validation.${validationError}`)}

- ) : sameAsFailedCron && !error ? ( + ) : sameAsFailedCron ? (

{t("localSchedules.failedUpdateRetry")}

- ) : error ? ( -
-

{error}

-

- {t("localSchedules.updateFailureCaution")} -

-
) : null}
diff --git a/src/features/automations/ui/LocalSchedulesPanel.test.tsx b/src/features/automations/ui/LocalSchedulesPanel.test.tsx index 055d6755a..4d34fd9f4 100644 --- a/src/features/automations/ui/LocalSchedulesPanel.test.tsx +++ b/src/features/automations/ui/LocalSchedulesPanel.test.tsx @@ -138,7 +138,7 @@ describe("LocalSchedulesPanel", () => { await waitFor(() => expect(screen.getByText("0 10 * * 1-5")).toBeInTheDocument(), ); - expect(listLocalSchedules).toHaveBeenCalledTimes(2); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(2)); }); it("keeps the latest list response when a mutation resolves after a newer poll", async () => { @@ -274,16 +274,17 @@ describe("LocalSchedulesPanel", () => { expect(updateLocalScheduleCron).toHaveBeenCalledOnce(); }); - it("allows an expression again after another update succeeds", async () => { + 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]) - .mockResolvedValueOnce([schedule]) + .mockReturnValueOnce(reconciliation.promise) .mockResolvedValueOnce([{ ...schedule, cron: "0 11 * * 1-5" }]); renderPanel(); @@ -297,11 +298,25 @@ describe("LocalSchedulesPanel", () => { 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.getByRole("status")).toHaveTextContent( + "Checking the schedule's current state", + ); + 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(), ); @@ -309,6 +324,7 @@ describe("LocalSchedulesPanel", () => { await waitFor(() => expect(updateLocalScheduleCron).toHaveBeenCalledTimes(2), ); + await waitFor(() => expect(listLocalSchedules).toHaveBeenCalledTimes(3)); await waitFor(() => expect(screen.getByText("0 11 * * 1-5")).toBeInTheDocument(), ); diff --git a/src/shared/i18n/locales/en/automations.json b/src/shared/i18n/locales/en/automations.json index c82f3fd65..aca46dbf5 100644 --- a/src/shared/i18n/locales/en/automations.json +++ b/src/shared/i18n/locales/en/automations.json @@ -53,6 +53,7 @@ "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.", diff --git a/src/shared/i18n/locales/es/automations.json b/src/shared/i18n/locales/es/automations.json index cfaf27306..06f0e0480 100644 --- a/src/shared/i18n/locales/es/automations.json +++ b/src/shared/i18n/locales/es/automations.json @@ -53,6 +53,7 @@ "cronLabel": "Expresion cron de {{id}}", "cronHelp": "Usa cinco campos (minuto hora dia-del-mes mes dia-de-la-semana) o seis campos con segundos al inicio. Goose usa la zona horaria de este equipo; si especificas dia-del-mes y dia-de-la-semana, ambos deben coincidir. Este editor admite comodines numericos, listas, rangos, pasos y nombres de dias. Las ediciones personalizadas deben repetirse cada mes o mas frecuentemente. Usa un comodin para el mes y para dia-del-mes o dia-de-la-semana. Goose podria quitar el temporizador anterior si una actualizacion falla inesperadamente; verifica la programacion despues de un error al guardar.", "saving": "Guardando…", + "reconcilingSchedule": "Comprobando el estado actual de la programacion…", "updateError": "No se pudo actualizar la programacion de Goose.", "updateFailureCaution": "Es posible que el programador haya quitado el temporizador anterior si la actualizacion fallo inesperadamente. Verifica la programacion en Goose antes de depender de ella.", "refreshAfterUpdateError": "La actualizacion de la lista tambien fallo, asi que se desconoce el estado actual de la programacion.", From 65787f9ced79af44b6775f2bd008616069792d58 Mon Sep 17 00:00:00 2001 From: Kyle De Freitas Date: Wed, 4 Nov 2026 13:02:15 -0400 Subject: [PATCH 4/4] fix: retain schedule uncertainty through recovery --- .../lib/localScheduleSpanish.test.ts | 83 ++++++ .../ui/LocalScheduleCronEditor.tsx | 168 +++++++----- .../ui/LocalSchedulesPanel.test.tsx | 245 +++++++++++++++++- src/shared/i18n/locales/en/automations.json | 3 + src/shared/i18n/locales/es/automations.json | 39 +-- 5 files changed, 444 insertions(+), 94 deletions(-) create mode 100644 src/features/automations/lib/localScheduleSpanish.test.ts 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 index 85a34cffb..f46bd5199 100644 --- a/src/features/automations/ui/LocalScheduleCronEditor.tsx +++ b/src/features/automations/ui/LocalScheduleCronEditor.tsx @@ -1,7 +1,17 @@ import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { + useIsMutating, + useMutation, + useQuery, + useQueryClient, +} from "@tanstack/react-query"; import { useTranslation } from "react-i18next"; -import { IconCheck, IconPencil, IconX } from "@tabler/icons-react"; +import { + IconAlertTriangle, + IconCheck, + IconPencil, + IconX, +} from "@tabler/icons-react"; import type { ScheduledJobDto } from "@aaif/goose-sdk"; import { LOCAL_SCHEDULES_QUERY_KEY, @@ -10,6 +20,7 @@ import { } 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"; @@ -33,21 +44,31 @@ export function LocalScheduleCronEditor({ const editorRef = useRef(null); const wasEditingRef = useRef(false); const restoreFocusAfterEditRef = useRef(false); - const [failedCrons, setFailedCrons] = useState>(() => new Set()); - const scheduleRefreshGenerationRef = useRef(0); + // 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.has(normalizedCron); + 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); - scheduleRefreshGenerationRef.current += 1; await queryClient.cancelQueries({ queryKey: LOCAL_SCHEDULES_QUERY_KEY }); }, onSuccess: async () => { @@ -55,9 +76,8 @@ export function LocalScheduleCronEditor({ queryKey: LOCAL_SCHEDULES_QUERY_KEY, }); setError(null); - setFailedCrons(new Set()); + queryClient.setQueryData(failureQueryKey, []); setIsSaving(false); - scheduleRefreshGenerationRef.current += 1; restoreFocusAfterEditRef.current = editorRef.current?.contains(document.activeElement) === true; setIsEditing(false); @@ -66,10 +86,11 @@ export function LocalScheduleCronEditor({ setError( formatAcpErrorMessage(mutationError, t("localSchedules.updateError")), ); - setFailedCrons((previous) => new Set(previous).add(variables.cron)); + queryClient.setQueryData(failureQueryKey, (previous = []) => [ + ...new Set([...previous, variables.cron]), + ]); setIsSaving(false); setIsReconciling(true); - const reconcileGeneration = ++scheduleRefreshGenerationRef.current; restoreFocusAfterEditRef.current = editorRef.current?.contains(document.activeElement) === true || document.activeElement === document.body; @@ -78,18 +99,14 @@ export function LocalScheduleCronEditor({ await queryClient.cancelQueries({ queryKey: LOCAL_SCHEDULES_QUERY_KEY, }); - // Read fresh state directly; don't join a poll that may have captured - // the pre-update schedule while the failed mutation was in flight. - const refreshedSchedules = await listLocalSchedules(); - await queryClient.cancelQueries({ + // 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, }); - if (scheduleRefreshGenerationRef.current === reconcileGeneration) { - queryClient.setQueryData( - LOCAL_SCHEDULES_QUERY_KEY, - refreshedSchedules, - ); - } } catch { setError((current) => current @@ -97,9 +114,7 @@ export function LocalScheduleCronEditor({ : t("localSchedules.refreshAfterUpdateError"), ); } finally { - if (scheduleRefreshGenerationRef.current === reconcileGeneration) { - setIsReconciling(false); - } + setIsReconciling(false); } }, }); @@ -110,6 +125,32 @@ export function LocalScheduleCronEditor({ !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(); @@ -149,6 +190,7 @@ export function LocalScheduleCronEditor({ function save() { if ( + isScheduleMutating || (isReconciling && !errorCoversCurrentDraft) || !hasChanged || (validationError && !hasKnownCron) || @@ -164,22 +206,32 @@ export function LocalScheduleCronEditor({ if (!isEditing) { return ( - +
0 ? "w-full space-y-2" : undefined}> + {uncertaintyWarning} + +
); } return (
+ {uncertaintyWarning} @@ -190,50 +242,27 @@ export function LocalScheduleCronEditor({ onChange={(event) => { setDraft(event.target.value); }} - aria-invalid={!!validationError || !!error} - aria-describedby={`local-schedule-cron-help-${schedule.id}`} + aria-invalid={displayedError !== null} + aria-describedby={displayedError ? `${helpId} ${errorId}` : helpId} disabled={isSaving} autoComplete="off" spellCheck={false} className="font-mono" /> -

+

{t("localSchedules.cronHelp")}

- {error && errorCoversCurrentDraft ? ( -
-

{error}

-

- {t("localSchedules.updateFailureCaution")} +

+ {displayedError ? ( +

+ {displayedError}

-
- ) : scheduleChangedElsewhere ? ( -

- {t("localSchedules.changedElsewhere")} -

- ) : validationError && !hasKnownCron ? ( -

- {t(`localSchedules.validation.${validationError}`)} -

- ) : isReconciling ? ( -

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

- ) : error ? ( -
-

{error}

-

- {t("localSchedules.updateFailureCaution")} + ) : isReconciling ? ( +

+ {t("localSchedules.reconcilingSchedule")}

-
- ) : sameAsFailedCron ? ( -

- {t("localSchedules.failedUpdateRetry")} -

- ) : null} + ) : null} +