From fa75cdd01cc1495903f10d41425612faf6b0b85e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 20 Aug 2026 09:02:48 -0700 Subject: [PATCH] fix(ui): validate automation condition triggers (#126718) --- ui/src/e2e/cron-trigger-authoring.e2e.test.ts | 165 ++++++++++++++++ ui/src/i18n/locales/en.ts | 4 + ui/src/lib/cron/index.test.ts | 184 +++++++++++++++++- ui/src/lib/cron/index.ts | 10 +- ui/src/pages/cron/view.test.ts | 55 ++++++ ui/src/pages/cron/view.ts | 11 +- 6 files changed, 422 insertions(+), 7 deletions(-) create mode 100644 ui/src/e2e/cron-trigger-authoring.e2e.test.ts diff --git a/ui/src/e2e/cron-trigger-authoring.e2e.test.ts b/ui/src/e2e/cron-trigger-authoring.e2e.test.ts new file mode 100644 index 000000000000..9f867c75cc0b --- /dev/null +++ b/ui/src/e2e/cron-trigger-authoring.e2e.test.ts @@ -0,0 +1,165 @@ +// Real-Chromium coverage keeps automation condition authoring aligned with Gateway contracts. +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import type { Page } from "playwright"; +import { expect, it } from "vitest"; +import { installMockGateway } from "../test-helpers/control-ui-e2e.ts"; +import { createControlUiE2eSuite } from "./control-ui-e2e-suite.test-support.ts"; + +const suite = createControlUiE2eSuite({ + name: "Control UI automation condition-trigger authoring", + startServerBeforeBrowser: true, + unavailableMessage: (executablePath) => `Playwright Chromium is unavailable at ${executablePath}`, +}); + +const proofDirectory = process.env.OPENCLAW_TRIGGER_UI_PROOF_DIR; +const proofStage = process.env.OPENCLAW_TRIGGER_UI_PROOF_STAGE ?? "after"; + +const scriptJob = { + id: "existing-script-automation", + configRevision: "existing-script-revision", + name: "Script health check", + enabled: true, + createdAtMs: Date.parse("2026-05-29T08:00:00.000Z"), + updatedAtMs: Date.parse("2026-05-29T08:05:00.000Z"), + schedule: { kind: "every", everyMs: 60_000 }, + sessionTarget: "isolated", + wakeMode: "next-heartbeat", + payload: { kind: "script", script: "return { ready: true };" }, + state: {}, +}; + +function listResponse(jobs: unknown[]) { + return { + jobs, + snapshotRevision: "trigger-authoring-fixture", + total: jobs.length, + offset: 0, + limit: 50, + hasMore: false, + nextOffset: null, + }; +} + +async function captureProof(page: Page, name: string) { + if (!proofDirectory) { + return; + } + await mkdir(proofDirectory, { recursive: true }); + await page.screenshot({ + animations: "disabled", + path: path.join(proofDirectory, `${proofStage}-${name}.png`), + }); +} + +async function selectSeconds(page: Page) { + const unit = page.locator("wa-select").filter({ + has: page.locator('[slot="label"]', { hasText: "Unit" }), + }); + await unit.click(); + await page.getByRole("option", { name: "Seconds", exact: true }).click(); +} + +suite.define(() => { + it("prevents unsupported condition triggers while preserving valid interval submissions", async () => { + await suite.withPage( + { locale: "en-US", serviceWorkers: "block", viewport: { height: 1_050, width: 1_440 } }, + async ({ page }) => { + const gateway = await installMockGateway(page, { + methodResponses: { + "cron.add": { id: "new-automation" }, + "cron.list": { + cases: [ + { match: { lastRunStatus: "error" }, response: listResponse([]) }, + { response: listResponse([scriptJob]) }, + ], + }, + "cron.runs": { + entries: [], + total: 0, + offset: 0, + limit: 50, + hasMore: false, + nextOffset: null, + }, + "cron.status": { enabled: true, jobs: 1, nextWakeAtMs: null }, + }, + }); + + await page.goto(`${suite.server.baseUrl}cron`); + await page.locator('[data-test-id="cron-row-existing-script-automation"]').click(); + await page.locator("details.cron-advanced > summary").click(); + + const scriptTriggerControlCount = await page + .locator("wa-switch.settings-toggle") + .filter({ hasText: "Condition trigger" }) + .count(); + await page + .getByText("Condition trigger", { exact: true }) + .evaluate((element) => element.scrollIntoView({ block: "center" })); + await captureProof(page, "01-script-payload-condition-control"); + + await page.locator('[data-test-id="cron-back"]').click(); + await page.locator('[data-test-id="cron-new-task"]').click(); + await page.locator("#cron-name").fill("Conditional interval"); + await page.locator("#cron-payload-text").fill("Run when the condition matches"); + await selectSeconds(page); + await page.locator("#cron-every-amount").fill("5"); + await page.locator("details.cron-advanced > summary").click(); + await page + .locator(".settings-row--toggle") + .filter({ hasText: "Condition trigger" }) + .click(); + await page.locator("#cron-trigger-script").fill("json({ fire: true })"); + await page + .locator("#cron-every-amount") + .evaluate((element) => element.scrollIntoView({ block: "center" })); + await captureProof(page, "02-triggered-five-second-validation"); + + expect(scriptTriggerControlCount).toBe(0); + + const intervalError = page.locator("#cron-error-everyAmount"); + await intervalError.waitFor({ state: "visible" }); + expect(await intervalError.textContent()).toMatch(/30/); + expect(await page.locator("#cron-every-amount").getAttribute("aria-invalid")).toBe("true"); + expect(await page.locator('[data-test-id="cron-submit"]').isDisabled()).toBe(true); + expect(await gateway.getRequests("cron.add")).toHaveLength(0); + + await page.locator("#cron-every-amount").fill("30"); + await expect.poll(async () => intervalError.count()).toBe(0); + expect(await page.locator('[data-test-id="cron-submit"]').isEnabled()).toBe(true); + await captureProof(page, "03-triggered-thirty-second-boundary"); + await page.locator('[data-test-id="cron-submit"]').click(); + + const triggeredRequest = await gateway.waitForRequest("cron.add"); + expect(triggeredRequest.params).toMatchObject({ + name: "Conditional interval", + schedule: { kind: "every", everyMs: 30_000 }, + trigger: { script: "json({ fire: true })", once: false }, + }); + await expect.poll(async () => page.locator('[data-test-id="cron-submit"]').count()).toBe(0); + + await page.locator('[data-test-id="cron-new-task"]').click(); + await page.locator("#cron-name").fill("Unconditional interval"); + await page.locator("#cron-payload-text").fill("Run every five seconds"); + await selectSeconds(page); + await page.locator("#cron-every-amount").fill("5"); + + expect(await page.locator("#cron-error-everyAmount").count()).toBe(0); + expect(await page.locator('[data-test-id="cron-submit"]').isEnabled()).toBe(true); + await captureProof(page, "04-untriggered-five-second-interval"); + + const previousAdds = (await gateway.getRequests("cron.add")).length; + await page.locator('[data-test-id="cron-submit"]').click(); + const untriggeredRequest = await gateway.waitForRequest("cron.add", { + after: previousAdds, + }); + expect(untriggeredRequest.params).toMatchObject({ + name: "Unconditional interval", + schedule: { kind: "every", everyMs: 5_000 }, + }); + expect(untriggeredRequest.params).not.toHaveProperty("trigger"); + }, + ); + }); +}); diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 4e234e2399d8..9dbc24db7efc 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -6595,9 +6595,13 @@ export const en: TranslationMap = { nameRequired: "Name is required.", scheduleAtInvalid: "Enter a valid date/time.", everyAmountInvalid: "Interval must be greater than 0.", + triggerIntervalTooShort: + "Condition-triggered automations must run at least every 30 seconds.", cronExprRequired: "Cron expression is required.", staggerAmountInvalid: "Stagger must be greater than 0.", triggerScriptRequired: "Trigger script is required when the condition trigger is enabled.", + triggerScriptPayloadUnsupported: + "Script payloads cannot use condition triggers because both own the same saved state.", triggerScheduleUnsupported: "Condition triggers require an interval, cron, or stream schedule.", systemTextRequired: "System text is required.", diff --git a/ui/src/lib/cron/index.test.ts b/ui/src/lib/cron/index.test.ts index 75dda6b4afbf..b6ea6c24bad2 100644 --- a/ui/src/lib/cron/index.test.ts +++ b/ui/src/lib/cron/index.test.ts @@ -153,7 +153,7 @@ function createCronEditHarness(job: CronJob) { await addCronJob(state); return findRequestCall(request.mock.calls, "cron.update"); }; - return { state, submit }; + return { request, state, submit }; } const requireRecord = createRequireRecord("record", "expected-label-record"); @@ -1345,6 +1345,63 @@ describe("cron controller", () => { expect(requestPatch(call).trigger).toBeNull(); }); + it("requires an explicit clear before saving an existing script payload with a condition trigger", async () => { + const job = createCronJob({ + id: "job-script-trigger-conflict", + name: "Conflicting script", + schedule: { kind: "every", everyMs: 30_000 }, + payload: { kind: "script", script: "json({ state: {} })" }, + trigger: { script: "json({ fire: true })" }, + delivery: { mode: "none" }, + }); + const { request, state, submit } = createCronEditHarness(job); + + expect(state.cronForm.triggerEnabled).toBe(true); + expect(await addCronJob(state)).toEqual({ saved: false }); + expect(state.cronFieldErrors.triggerScript).toBe("cron.errors.triggerScriptPayloadUnsupported"); + expect(request).not.toHaveBeenCalled(); + + state.cronForm.triggerEnabled = false; + const call = await submit(); + + expect(requestPatch(call).trigger).toBeNull(); + expect(requestPatch(call)).not.toHaveProperty("payload"); + }); + + it.each(["agentTurn", "systemEvent", "command"] as const)( + "preserves condition-trigger authoring for %s payloads", + (payloadKind) => { + expect( + validateCronForm({ + ...DEFAULT_CRON_FORM, + name: "Supported conditional automation", + payloadKind, + payloadLocked: payloadKind === "command", + payloadText: "run", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + }).triggerScript, + ).toBeUndefined(); + }, + ); + + it("revalidates condition triggers when the payload changes to or from script", () => { + const form = { + ...DEFAULT_CRON_FORM, + name: "Payload transition", + payloadText: "run", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + }; + + expect(validateCronForm({ ...form, payloadKind: "script", payloadLocked: true })).toMatchObject( + { + triggerScript: "cron.errors.triggerScriptPayloadUnsupported", + }, + ); + expect(validateCronForm({ ...form, payloadKind: "agentTurn" }).triggerScript).toBeUndefined(); + }); + it("sends lightContext=false in cron.update when clearing prior light-context setting", async () => { const job = createCronJob({ id: "job-clear-light", @@ -1712,6 +1769,107 @@ describe("cron controller", () => { }, ); + it.each([ + ["5", "seconds"], + ["29.999", "seconds"], + ["0.49", "minutes"], + ] as const)( + "rejects a condition-triggered interval below the Gateway minimum: %s %s", + async (everyAmount, everyUnit) => { + const request = createCronRequest("job-trigger-too-fast"); + const state = createStateWithRequest(request, { + cronForm: { + ...DEFAULT_CRON_FORM, + name: "Conditional automation", + everyAmount, + everyUnit, + payloadText: "run", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + deliveryMode: "none", + }, + }); + + expect(await addCronJob(state)).toEqual({ saved: false }); + expect(state.cronFieldErrors.everyAmount).toBe("cron.errors.triggerIntervalTooShort"); + expect(request).not.toHaveBeenCalled(); + }, + ); + + it.each([ + ["30", "seconds"], + ["0.5", "minutes"], + ] as const)( + "accepts a condition-triggered interval at the Gateway minimum: %s %s", + async (everyAmount, everyUnit) => { + const { submit } = createCronSubmitHarness("job-trigger-boundary", { + form: { + name: "Boundary automation", + everyAmount, + everyUnit, + payloadText: "run", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + deliveryMode: "none", + }, + }); + + const { call, result } = await submit(); + + expect(result.saved).toBe(true); + expect(requestPayload(call).schedule).toEqual({ kind: "every", everyMs: 30_000 }); + }, + ); + + it("blocks adding a condition trigger to an existing short-interval automation", async () => { + const job = createCronJob({ + id: "job-existing-short", + name: "Existing short interval", + schedule: { kind: "every", everyMs: 5_000 }, + }); + const { request, state } = createCronEditHarness(job); + state.cronForm.triggerEnabled = true; + state.cronForm.triggerScript = "json({ fire: true })"; + + expect(await addCronJob(state)).toEqual({ saved: false }); + expect(state.cronFieldErrors.everyAmount).toBe("cron.errors.triggerIntervalTooShort"); + expect(request).not.toHaveBeenCalled(); + }); + + it("blocks shortening an existing condition-triggered automation below the minimum", async () => { + const job = createCronJob({ + id: "job-shorten-triggered", + name: "Existing conditional automation", + schedule: { kind: "every", everyMs: 30_000 }, + trigger: { script: "json({ fire: true })" }, + }); + const { request, state } = createCronEditHarness(job); + state.cronForm.everyAmount = "5"; + + expect(await addCronJob(state)).toEqual({ saved: false }); + expect(state.cronFieldErrors.everyAmount).toBe("cron.errors.triggerIntervalTooShort"); + expect(request).not.toHaveBeenCalled(); + }); + + it("allows a short interval again when removing an existing condition trigger", async () => { + const job = createCronJob({ + id: "job-remove-short-trigger", + name: "Previously conditional automation", + schedule: { kind: "every", everyMs: 30_000 }, + trigger: { script: "json({ fire: true })" }, + }); + const { state, submit } = createCronEditHarness(job); + state.cronForm.everyAmount = "5"; + state.cronForm.triggerEnabled = false; + + const call = await submit(); + + expect(requestPatch(call)).toMatchObject({ + schedule: { kind: "every", everyMs: 5_000 }, + trigger: null, + }); + }); + it.each([ ["1.5", "minutes", 90_000], ["4.1", "minutes", 246_000], @@ -1861,6 +2019,30 @@ describe("cron controller", () => { ); }); + it("keeps an existing condition trigger when cloning a script into an editable agent task", async () => { + const request = createCronRequest("job-script-clone"); + const sourceJob = createCronJob({ + id: "job-script-source", + name: "Script source", + schedule: { kind: "every", everyMs: 30_000 }, + payload: { kind: "script", script: "json({ state: {} })" }, + trigger: { script: "json({ fire: true })" }, + delivery: { mode: "none" }, + }); + const state = createStateWithRequest(request, { cronJobs: [sourceJob] }); + + startCronClone(state, sourceJob); + state.cronForm.payloadText = "Continue as an agent task"; + + expect(state.cronForm.payloadKind).toBe("agentTurn"); + expect(state.cronForm.triggerEnabled).toBe(true); + expect(await addCronJob(state)).toEqual({ saved: true, jobId: "job-script-clone" }); + expect(requestPayload(findRequestCall(request.mock.calls, "cron.add"))).toMatchObject({ + payload: { kind: "agentTurn", message: "Continue as an agent task" }, + trigger: { script: "json({ fire: true })", once: false }, + }); + }); + it("round-trips hidden delivery destinations through clone and edit", async () => { const sourceJob = createCronJob({ id: "job-routing", diff --git a/ui/src/lib/cron/index.ts b/ui/src/lib/cron/index.ts index b4eed0ac09a1..0b8c9f5f1476 100644 --- a/ui/src/lib/cron/index.ts +++ b/ui/src/lib/cron/index.ts @@ -1,6 +1,7 @@ import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { sortUniqueStrings } from "@openclaw/normalization-core/string-normalization"; +import { resolveCronTriggerMinIntervalMs } from "../../../../src/config/cron-limits.js"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; import type { CronJob, @@ -336,8 +337,11 @@ export function validateCronForm(form: CronFormState): CronFieldErrors { errors.scheduleAt = "cron.errors.scheduleAtInvalid"; } } else if (form.scheduleKind === "every") { - if (parseCronEveryMs(form.everyAmount, form.everyUnit) === undefined) { + const everyMs = parseCronEveryMs(form.everyAmount, form.everyUnit); + if (everyMs === undefined) { errors.everyAmount = "cron.errors.everyAmountInvalid"; + } else if (form.triggerEnabled && everyMs < resolveCronTriggerMinIntervalMs()) { + errors.everyAmount = "cron.errors.triggerIntervalTooShort"; } } else if (form.scheduleKind === "cron") { if (!form.cronExpr.trim()) { @@ -354,7 +358,9 @@ export function validateCronForm(form: CronFormState): CronFieldErrors { } } if (form.triggerEnabled) { - if ( + if (form.payloadKind === "script") { + errors.triggerScript = "cron.errors.triggerScriptPayloadUnsupported"; + } else if ( form.scheduleKind !== "every" && form.scheduleKind !== "cron" && form.scheduleKind !== "stream" diff --git a/ui/src/pages/cron/view.test.ts b/ui/src/pages/cron/view.test.ts index 0f6984b24302..22486a8ad64a 100644 --- a/ui/src/pages/cron/view.test.ts +++ b/ui/src/pages/cron/view.test.ts @@ -741,6 +741,60 @@ describe("cron view editor", () => { ); expect(container.textContent).toContain("contents stay read-only"); expect(container.querySelector('option[value="script"]')).toBeNull(); + expect(findToggleByLabel(container, "Condition trigger")).toBeNull(); + expect(container.textContent).toContain("Script payloads cannot use condition triggers"); + }); + + it("keeps an incompatible existing script condition trigger visible and explicitly clearable", () => { + const onFormChange = vi.fn(); + const job = createJob("job-script-trigger", { + payload: { kind: "script", script: "json({ state: {} })" }, + trigger: { script: "json({ fire: true })" }, + }); + const container = renderView({ + jobs: [job], + editingJob: job, + onFormChange, + form: { + ...DEFAULT_CRON_FORM, + name: job.name, + payloadKind: "script", + payloadLocked: true, + payloadText: "json({ state: {} })", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + }, + fieldErrors: { triggerScript: "cron.errors.triggerScriptPayloadUnsupported" }, + canSubmit: false, + }); + + expect(findToggleByLabel(container, "Condition trigger")).toBeNull(); + expect(container.querySelector("#cron-trigger-script")).toBeNull(); + expect(container.textContent).toContain("Script payloads cannot use condition triggers"); + getButtonByText(container, "Clear trigger").click(); + expect(onFormChange).toHaveBeenCalledWith({ triggerEnabled: false }); + }); + + it("attaches the triggered minimum-interval error to the visible recurring interval", () => { + const container = renderView({ + createOpen: true, + canSubmit: false, + form: { + ...DEFAULT_CRON_FORM, + everyAmount: "5", + everyUnit: "seconds", + triggerEnabled: true, + triggerScript: "json({ fire: true })", + }, + fieldErrors: { everyAmount: "cron.errors.triggerIntervalTooShort" }, + }); + + const interval = getElement(container, "#cron-every-amount", HTMLInputElement); + expect(interval.getAttribute("aria-invalid")).toBe("true"); + expect(interval.getAttribute("aria-describedby")).toBe("cron-error-everyAmount"); + expect(container.querySelector("#cron-error-everyAmount")?.textContent).toContain( + "at least every 30 seconds", + ); }); it("highlights locked command payloads as shell and keeps heartbeat payloads plain", () => { @@ -762,6 +816,7 @@ describe("cron view editor", () => { const payload = getElement(command, "#cron-payload-text", HTMLPreElement); expect(payload.textContent).toBe("echo $HOME"); expect(payload.querySelector(".hljs-built_in")?.textContent).toBe("echo"); + expect(findToggleByLabel(command, "Condition trigger")).not.toBeNull(); const heartbeat = renderView({ jobs: [job], diff --git a/ui/src/pages/cron/view.ts b/ui/src/pages/cron/view.ts index a4a2d2fe9758..4a6744deb8c1 100644 --- a/ui/src/pages/cron/view.ts +++ b/ui/src/pages/cron/view.ts @@ -1728,12 +1728,15 @@ function renderAdvanced( } function renderTriggerRows(props: CronProps) { - if (!props.triggersEnabled) { + const scriptPayload = props.form.payloadKind === "script"; + if (!props.triggersEnabled || scriptPayload) { return renderSettingsRow({ title: t("cron.form.conditionTrigger"), - description: props.form.triggerEnabled - ? t("cron.form.triggerDisabledConfigured") - : t("cron.form.triggerDisabled"), + description: scriptPayload + ? t("cron.errors.triggerScriptPayloadUnsupported") + : props.form.triggerEnabled + ? t("cron.form.triggerDisabledConfigured") + : t("cron.form.triggerDisabled"), control: props.form.triggerEnabled ? html`