fix(webui): allow content-only automation edits
Avoid resubmitting unchanged schedules from the automation edit dialog so completed one-time automations can still have their content updated. Treat unchanged schedules as already-valid on the backend while preserving validation for actual schedule changes.
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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={
|
||||
|
||||
@@ -3802,6 +3802,7 @@ type AutomationEditDraft = {
|
||||
tz: string;
|
||||
atLocal: string;
|
||||
};
|
||||
type AutomationScheduleUpdate = NonNullable<AutomationUpdatePayload["schedule"]>;
|
||||
|
||||
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<HTMLFormElement>) => {
|
||||
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, unknown>) => 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 {
|
||||
|
||||
@@ -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(<App />);
|
||||
|
||||
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<string, string>;
|
||||
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({
|
||||
|
||||
Reference in New Issue
Block a user