diff --git a/nanobot/webui/ws_http.py b/nanobot/webui/ws_http.py index 99b1f92e..2fc00ec8 100644 --- a/nanobot/webui/ws_http.py +++ b/nanobot/webui/ws_http.py @@ -23,7 +23,7 @@ from websockets.http11 import Request as WsRequest from websockets.http11 import Response from nanobot.command.builtin import builtin_command_palette -from nanobot.cron.types import CronSchedule +from nanobot.cron.types import CronJob, CronSchedule from nanobot.utils.subagent_channel_display import scrub_subagent_messages_for_channel from nanobot.webui.file_preview import WebUIFilePreviewError, file_preview_payload from nanobot.webui.gateway_tokens import GatewayTokenStore, token_response_payload @@ -600,7 +600,7 @@ class GatewayHTTPHandler: values = _automation_values_from_request(request) if values is None: return _http_error(400, "invalid automation update payload") - parsed = _parse_automation_update(values) + parsed = _parse_automation_update(values, current_job=job) if isinstance(parsed, str): return _http_error(400, parsed) try: @@ -786,7 +786,11 @@ def _automation_values_from_request(request: WsRequest) -> dict[str, Any] | None return values if isinstance(values, dict) else None -def _parse_automation_update(values: dict[str, Any]) -> dict[str, Any] | str: +def _parse_automation_update( + values: dict[str, Any], + *, + current_job: CronJob | None = None, +) -> dict[str, Any] | str: update: dict[str, Any] = {} if "name" in values: raw_name = values.get("name") @@ -811,6 +815,8 @@ def _parse_automation_update(values: dict[str, Any]) -> dict[str, Any] | str: parsed_schedule = _parse_automation_schedule(raw_schedule) if isinstance(parsed_schedule, str): return parsed_schedule + if current_job is not None and _schedule_matches_job(parsed_schedule, current_job): + return update schedule_error = _validate_automation_schedule(parsed_schedule) if schedule_error: return schedule_error @@ -849,6 +855,21 @@ def _parse_automation_schedule(values: dict[str, Any]) -> CronSchedule | str: return "unknown schedule kind" +def _schedule_matches_job(schedule: CronSchedule, job: CronJob) -> bool: + current = job.schedule + if schedule.kind != current.kind: + return False + if schedule.kind == "at": + return schedule.at_ms == current.at_ms + if schedule.kind == "every": + return schedule.every_ms == current.every_ms + if schedule.kind == "cron": + return (schedule.expr or "") == (current.expr or "") and ( + schedule.tz or None + ) == (current.tz or None) + return False + + def _validate_automation_schedule(schedule: CronSchedule) -> str | None: if schedule.kind == "at": if not schedule.at_ms or schedule.at_ms <= int(time.time() * 1000): diff --git a/tests/channels/test_websocket_http_routes.py b/tests/channels/test_websocket_http_routes.py index d08681d4..6a8eaf12 100644 --- a/tests/channels/test_websocket_http_routes.py +++ b/tests/channels/test_websocket_http_routes.py @@ -856,6 +856,15 @@ async def test_webui_automations_route_lists_all_jobs_and_allows_user_actions( origin_channel="weixin", origin_chat_id="wx-chat", ) + past_one_shot_job = cron.add_job( + name="Past one-shot", + schedule=CronSchedule(kind="at", at_ms=1), + message="Old one-shot message", + session_key="websocket:abc", + origin_channel="websocket", + origin_chat_id="abc", + delete_after_run=True, + ) cron.register_system_job( CronJob( id="heartbeat", @@ -955,6 +964,22 @@ async def test_webui_automations_route_lists_all_jobs_and_allows_user_actions( assert invalid_cron_update.status_code == 400 assert cron.get_job(user_job.id).schedule.expr == "0 9 * * *" + past_one_shot_update = await _http_get( + f"{base_url}/api/webui/automations/update?id={past_one_shot_job.id}", + headers={ + **auth, + "X-Nanobot-Automation-Values": json.dumps( + { + "message": "Updated one-shot message", + "schedule": {"kind": "at", "at_ms": 1}, + } + ), + }, + ) + assert past_one_shot_update.status_code == 200 + assert cron.get_job(past_one_shot_job.id).payload.message == "Updated one-shot message" + assert cron.get_job(past_one_shot_job.id).schedule.at_ms == 1 + protected_update = await _http_get( f"{base_url}/api/webui/automations/update?id=heartbeat", headers={ diff --git a/webui/src/components/settings/SettingsView.tsx b/webui/src/components/settings/SettingsView.tsx index 5dc00e14..49a27ba8 100644 --- a/webui/src/components/settings/SettingsView.tsx +++ b/webui/src/components/settings/SettingsView.tsx @@ -3802,6 +3802,7 @@ type AutomationEditDraft = { tz: string; atLocal: string; }; +type AutomationScheduleUpdate = NonNullable; const AUTOMATION_EVERY_UNITS: Array<{ value: AutomationEveryUnit; ms: number }> = [ { value: "second", ms: 1000 }, @@ -3830,7 +3831,7 @@ function AutomationEditDialog({ setDraft(automationDraftFromJob(job)); }, [job]); - const validation = automationEditDraftError(draft, tx); + const validation = automationEditDraftError(draft, job, tx); const scheduleOptions = [ { value: "every", label: tx("settings.automations.scheduleTypes.every", "Interval") }, { value: "cron", label: tx("settings.automations.scheduleTypes.cron", "Cron") }, @@ -3845,7 +3846,7 @@ function AutomationEditDialog({ const submit = (event: FormEvent) => { event.preventDefault(); - const payload = automationUpdatePayloadFromDraft(draft); + const payload = automationUpdatePayloadFromDraft(draft, job); if (!job || typeof payload === "string") return; void onSave(job, payload); }; @@ -4143,6 +4144,7 @@ function formatLocalDateTimeInput(ms: number): string { function automationEditDraftError( draft: AutomationEditDraft, + job: SessionAutomationJob | null, tx: (key: string, fallback: string, values?: Record) => string, ): string | null { if (!draft.name.trim()) return tx("settings.automations.validation.nameRequired", "Name is required."); @@ -4163,33 +4165,58 @@ function automationEditDraftError( if (!Number.isFinite(atMs)) { return tx("settings.automations.validation.timeRequired", "Run time is required."); } - if (atMs <= Date.now()) { + if (atMs <= Date.now() && automationScheduleChanged(draft, job)) { return tx("settings.automations.validation.futureRequired", "Run time must be in the future."); } } return null; } -function automationUpdatePayloadFromDraft(draft: AutomationEditDraft): AutomationUpdatePayload | string { +function automationUpdatePayloadFromDraft( + draft: AutomationEditDraft, + job: SessionAutomationJob | null, +): AutomationUpdatePayload | string { const name = draft.name.trim(); const message = draft.message.trim(); if (!name || !message) return "invalid"; const payload: AutomationUpdatePayload = { name, message }; + const schedule = automationSchedulePayloadFromDraft(draft); + if (typeof schedule === "string") return schedule; + if (automationScheduleChanged(draft, job, schedule)) { + payload.schedule = schedule; + } + return payload; +} + +function automationSchedulePayloadFromDraft(draft: AutomationEditDraft): AutomationScheduleUpdate | string { if (draft.scheduleKind === "every") { const unit = AUTOMATION_EVERY_UNITS.find((candidate) => candidate.value === draft.everyUnit); const value = Number(draft.everyValue); if (!unit || !Number.isInteger(value) || value <= 0) return "invalid"; - payload.schedule = { kind: "every", every_ms: value * unit.ms }; + return { kind: "every", every_ms: value * unit.ms }; } else if (draft.scheduleKind === "cron") { const expr = draft.cronExpr.trim(); if (!expr) return "invalid"; - payload.schedule = { kind: "cron", expr, ...(draft.tz.trim() ? { tz: draft.tz.trim() } : {}) }; + return { kind: "cron", expr, ...(draft.tz.trim() ? { tz: draft.tz.trim() } : {}) }; } else { const atMs = new Date(draft.atLocal).getTime(); if (!Number.isFinite(atMs)) return "invalid"; - payload.schedule = { kind: "at", at_ms: atMs }; + return { kind: "at", at_ms: atMs }; } - return payload; +} + +function automationScheduleChanged( + draft: AutomationEditDraft, + job: SessionAutomationJob | null, + schedule: AutomationScheduleUpdate | string = automationSchedulePayloadFromDraft(draft), +): boolean { + if (!job || typeof schedule === "string") return true; + if (schedule.kind !== job.schedule.kind) return true; + if (schedule.kind === "every") return schedule.every_ms !== job.schedule.every_ms; + if (schedule.kind === "cron") { + return schedule.expr !== (job.schedule.expr ?? "") || (schedule.tz ?? null) !== (job.schedule.tz ?? null); + } + return draft.atLocal !== formatLocalDateTimeInput(job.schedule.at_ms ?? NaN); } function automationSearchText(job: SessionAutomationJob): string { diff --git a/webui/src/tests/app-layout.test.tsx b/webui/src/tests/app-layout.test.tsx index 8303fe20..ccf3013a 100644 --- a/webui/src/tests/app-layout.test.tsx +++ b/webui/src/tests/app-layout.test.tsx @@ -452,6 +452,78 @@ describe("App layout", () => { expect(document.title).toBe("Automations ยท nanobot"); }); + it("edits a past one-time automation without resubmitting its old schedule", async () => { + const pastOneShot = { + id: "past-one-shot", + name: "Past one-shot", + enabled: true, + protected: false, + delete_after_run: true, + schedule: { kind: "at", at_ms: 1 }, + payload: { + message: "Old one-shot message", + kind: "agent_turn", + session_key: "websocket:chat-a", + }, + state: { + next_run_at_ms: null, + last_status: "ok", + pending: false, + run_history: [], + }, + origin: { + session_key: "websocket:chat-a", + channel: "websocket", + chat_id: "chat-a", + title: "Release prep", + preview: "Check release blockers", + }, + }; + mockFetchRoutes({ + "/api/settings": baseSettingsPayload(), + "/api/webui/automations": { jobs: [pastOneShot] }, + "/api/webui/automations/update?id=past-one-shot": { + jobs: [ + { + ...pastOneShot, + payload: { ...pastOneShot.payload, message: "Updated one-shot message" }, + }, + ], + }, + }); + + render(); + + await waitFor(() => expect(connectSpy).toHaveBeenCalled()); + const sidebar = screen.getByRole("navigation", { name: "Sidebar navigation" }); + fireEvent.click(within(sidebar).getByRole("button", { name: "Automations" })); + + expect(await screen.findByText("Past one-shot")).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Edit" })); + expect(screen.queryByText("Run time must be in the future.")).not.toBeInTheDocument(); + + fireEvent.change(screen.getByDisplayValue("Old one-shot message"), { + target: { value: "Updated one-shot message" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + + await waitFor(() => { + expect(fetch).toHaveBeenCalledWith( + "/api/webui/automations/update?id=past-one-shot", + expect.any(Object), + ); + }); + const updateCall = vi.mocked(fetch).mock.calls.find( + ([url]) => String(url) === "/api/webui/automations/update?id=past-one-shot", + ); + expect(updateCall).toBeTruthy(); + const headers = updateCall?.[1]?.headers as Record; + expect(JSON.parse(headers["X-Nanobot-Automation-Values"])).toEqual({ + name: "Past one-shot", + message: "Updated one-shot message", + }); + }); + it("localizes the Automations surface", async () => { await i18n.changeLanguage("zh-CN"); mockFetchRoutes({