[CCOR-13383]fix:task definition form missing fields

ui-next/src/pages/definition/task/TaskDefinitionButtons.tsx

@@ -9,7 +9,10 @@ import TrashIcon from "components/icons/TrashIcon";
 import XCloseIcon from "components/icons/XCloseIcon";
 import fastDeepEqual from "fast-deep-equal";
 import { TaskDefinitionFormMachineEvent } from "pages/definition/task/form/state/types";
-import { TASK_FORM_MACHINE_ID } from "pages/definition/task/state/helpers";
+import {
+  TASK_FORM_MACHINE_ID,
+  isSaveDisabled,
+} from "pages/definition/task/state/helpers";
 import { useTaskDefinition } from "pages/definition/task/state/hook";
 import {
   TaskDefinitionButtonsProps,
@@ -41,18 +44,14 @@ const withFormState =
       [modifiedTaskDefinition, originTaskDefinition],
     );
     const isReset = buttonProps?.role === "reset";
-    const resetDisabledConditions = noChanges;
-    const saveDisabledConditions =
-      (!isNewTaskDef && noChanges) || isTrialExpired;
-    const noDescription = !(modifiedTaskDefinition.description ?? "").trim();
 
     return (
       <ButtonComponent
         {...buttonProps}
         disabled={
           isReset
-            ? resetDisabledConditions
-            : saveDisabledConditions || noDescription
+            ? noChanges
+            : isSaveDisabled({ noChanges, isNewTaskDef, isTrialExpired })
         }
       />
     );
@@ -82,18 +81,19 @@ const withEditorState =
     );
 
     const isReset = buttonProps?.role === "reset";
-    const resetDisabledConditions = noChanges;
-    const saveDisabledConditions =
-      jsonInvalid || (!isNewTaskDef && noChanges) || isTrialExpired;
-    const noDescription = !(modifiedTaskDefinition.description ?? "").trim();
 
     return (
       <ButtonComponent
         {...buttonProps}
         disabled={
           isReset
-            ? resetDisabledConditions
-            : saveDisabledConditions || noDescription
+            ? noChanges
+            : isSaveDisabled({
+                noChanges,
+                isNewTaskDef,
+                isTrialExpired,
+                jsonInvalid,
+              })
         }
       />
     );

ui-next/src/pages/definition/task/form/TaskDefinitionForm.test.tsx

@@ -0,0 +1,150 @@
+/**
+ * The Task tab must expose every field the Code tab's JSON carries, otherwise
+ * a setting can only be reached by hand-editing raw JSON. These tests cover
+ * the four that had no control: maxRetryDelaySeconds, backoffJitterMs,
+ * totalTimeoutSeconds and taskStatusListenerEnabled.
+ *
+ * They assert against the form machine's context, which is what the Code tab
+ * renders — so a control wired to a misspelled field name fails here. The
+ * compiler cannot catch that: modifiedTaskDefinition reaches the form through
+ * a loosely typed xstate ActorRef and is effectively `any`.
+ */
+import "@testing-library/jest-dom";
+import { fireEvent, render, screen } from "@testing-library/react";
+import { interpret } from "xstate";
+import { Provider as ThemeProvider } from "theme/material/provider";
+import TaskDefinitionForm from "pages/definition/task/form/TaskDefinitionForm";
+import { taskDefinitionFormMachine } from "pages/definition/task/form/state/machine";
+import { TaskRetryLogic, TaskTimeoutPolicy } from "pages/definition/task/state";
+import { TaskDefinitionDto } from "types/TaskDefinition";
+
+vi.mock("utils/query", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("utils/query")>()),
+  useFetch: () => ({ data: [], refetch: vi.fn() }),
+}));
+
+const baseTaskDefinition = {
+  name: "my_task",
+  description: "",
+  retryCount: 3,
+  retryDelaySeconds: 60,
+  retryLogic: TaskRetryLogic.FIXED,
+  backoffScaleFactor: 1,
+  timeoutSeconds: 3600,
+  timeoutPolicy: TaskTimeoutPolicy.TIME_OUT_WF,
+  responseTimeoutSeconds: 600,
+  pollTimeoutSeconds: 3600,
+  rateLimitPerFrequency: 0,
+  rateLimitFrequencyInSeconds: 1,
+  concurrentExecLimit: 0,
+  inputKeys: [],
+  outputKeys: [],
+  inputTemplate: {},
+} as unknown as TaskDefinitionDto;
+
+const renderForm = (overrides: Partial<TaskDefinitionDto> = {}) => {
+  const taskDefinition = {
+    ...baseTaskDefinition,
+    ...overrides,
+  } as TaskDefinitionDto;
+  const service = interpret(
+    taskDefinitionFormMachine.withContext({
+      modifiedTaskDefinition: taskDefinition,
+      originTaskDefinition: taskDefinition,
+    }),
+  ).start();
+
+  render(
+    <ThemeProvider>
+      <TaskDefinitionForm formActor={service as never} />
+    </ThemeProvider>,
+  );
+
+  return {
+    /** What the Code tab would show. */
+    definition: () =>
+      service.getSnapshot().context.modifiedTaskDefinition as Record<
+        string,
+        unknown
+      >,
+    json: () =>
+      (
+        service.getSnapshot().context as unknown as {
+          modifiedTaskDefinitionString?: string;
+        }
+      ).modifiedTaskDefinitionString ?? "",
+  };
+};
+
+const field = (label: string) =>
+  screen.getByLabelText(label) as HTMLInputElement;
+
+const setNumber = (label: string, value: string) =>
+  fireEvent.change(field(label), { target: { value } });
+
+describe("TaskDefinitionForm — fields that were JSON-only", () => {
+  it("shows the stored values for all four fields", () => {
+    renderForm({
+      maxRetryDelaySeconds: 120,
+      backoffJitterMs: 250,
+      totalTimeoutSeconds: 7200,
+      taskStatusListenerEnabled: false,
+    });
+
+    expect(field("Max retry delay seconds")).toHaveValue("120");
+    expect(field("Backoff jitter ms")).toHaveValue("250");
+    expect(field("Total Timeout Seconds")).toHaveValue("7200");
+    expect(
+      screen.getByLabelText("Enable task status listener"),
+    ).not.toBeChecked();
+  });
+
+  it("writes maxRetryDelaySeconds back into the definition", () => {
+    const { definition, json } = renderForm({ maxRetryDelaySeconds: 0 });
+
+    setNumber("Max retry delay seconds", "45");
+
+    expect(definition().maxRetryDelaySeconds).toBe(45);
+    expect(json()).toContain('"maxRetryDelaySeconds": 45');
+  });
+
+  it("writes backoffJitterMs back into the definition", () => {
+    const { definition } = renderForm({ backoffJitterMs: 0 });
+
+    setNumber("Backoff jitter ms", "500");
+
+    expect(definition().backoffJitterMs).toBe(500);
+  });
+
+  it("writes totalTimeoutSeconds back into the definition", () => {
+    const { definition } = renderForm({ totalTimeoutSeconds: 0 });
+
+    setNumber("Total Timeout Seconds", "900");
+
+    expect(definition().totalTimeoutSeconds).toBe(900);
+  });
+
+  it("toggles taskStatusListenerEnabled", () => {
+    const { definition } = renderForm({ taskStatusListenerEnabled: true });
+
+    fireEvent.click(screen.getByLabelText("Enable task status listener"));
+
+    expect(definition().taskStatusListenerEnabled).toBe(false);
+  });
+
+  it("reads an absent taskStatusListenerEnabled as on, matching the server default", () => {
+    renderForm();
+
+    expect(screen.getByLabelText("Enable task status listener")).toBeChecked();
+  });
+
+  it("keeps the retry delay cap and jitter editable under a FIXED retry policy", () => {
+    // Both are applied by the server for every retry policy, unlike the
+    // backoff scale factor, which only applies to the backoff policies.
+    renderForm({ retryLogic: TaskRetryLogic.FIXED });
+
+    expect(field("Max retry delay seconds")).toBeEnabled();
+    expect(field("Backoff jitter ms")).toBeEnabled();
+    expect(field("Backoff scale factor")).toBeDisabled();
+  });
+});

ui-next/src/pages/definition/task/form/TaskDefinitionForm.tsx

@@ -111,6 +111,28 @@ const TaskDefinitionForm = ({ formActor }: TaskDefinitionFormProps) => {
                 }}
               />
             </Grid>
+            <Grid size={12}>
+              <FormControlLabel
+                id="task-taskStatusListenerEnabled-field"
+                // The server defaults this on, so an absent value reads as on.
+                checked={
+                  modifiedTaskDefinition.taskStatusListenerEnabled ?? true
+                }
+                control={
+                  <Switch
+                    color="primary"
+                    style={{ marginRight: 8 }}
+                    onChange={({ target: { checked } }) =>
+                      handleChangeInputForm(
+                        "taskStatusListenerEnabled",
+                        checked,
+                      )
+                    }
+                  />
+                }
+                label="Enable task status listener"
+              />
+            </Grid>
           </Grid>
         </Grid>
       </Grid>
@@ -273,6 +295,48 @@ const TaskDefinitionForm = ({ formActor }: TaskDefinitionFormProps) => {
             placeholder="0"
           />
         </Grid>
+        <Grid size={12}>
+          <ConductorInputNumber
+            id="task-maxRetryDelaySeconds-field"
+            label="Max retry delay seconds"
+            fullWidth
+            name="maxRetryDelaySeconds"
+            onChange={handleChangeTaskForm}
+            value={modifiedTaskDefinition.maxRetryDelaySeconds}
+            error={!!error?.maxRetryDelaySeconds}
+            helperText={error?.maxRetryDelaySeconds?.message}
+            inputProps={{
+              allowNegative: false,
+            }}
+            tooltip={{
+              title: "Max retry delay seconds",
+              content:
+                "Upper limit (in seconds) on the delay between retries, capping whatever the retry policy computes. Applies to every retry policy. No cap if set to 0.",
+            }}
+            placeholder="0"
+          />
+        </Grid>
+        <Grid size={12}>
+          <ConductorInputNumber
+            id="task-backoffJitterMs-field"
+            label="Backoff jitter ms"
+            fullWidth
+            name="backoffJitterMs"
+            onChange={handleChangeTaskForm}
+            value={modifiedTaskDefinition.backoffJitterMs}
+            error={!!error?.backoffJitterMs}
+            helperText={error?.backoffJitterMs?.message}
+            inputProps={{
+              allowNegative: false,
+            }}
+            tooltip={{
+              title: "Backoff jitter ms",
+              content:
+                "A random delay between 0 and this value (in milliseconds) is added to each retry, spreading out retries that would otherwise fire together. No jitter if set to 0.",
+            }}
+            placeholder="0"
+          />
+        </Grid>
       </Grid>
       <Grid {...gridContainerItemProps}>
         <Grid size={12}>
@@ -322,6 +386,27 @@ const TaskDefinitionForm = ({ formActor }: TaskDefinitionFormProps) => {
             placeholder="3600"
           />
         </Grid>
+        <Grid size={12}>
+          <ConductorInputNumber
+            id="task-totalTimeoutSeconds-field"
+            fullWidth
+            label="Total Timeout Seconds"
+            name="totalTimeoutSeconds"
+            onChange={handleChangeTaskForm}
+            value={modifiedTaskDefinition.totalTimeoutSeconds}
+            error={!!error?.totalTimeoutSeconds}
+            helperText={error?.totalTimeoutSeconds?.message}
+            inputProps={{
+              allowNegative: false,
+            }}
+            tooltip={{
+              title: "Total timeout seconds",
+              content:
+                "Total time (in seconds) the task may take across every attempt, including the delays between retries, before it gets marked as TIMED_OUT. The timeout policy still applies. No limit if set to 0.",
+            }}
+            placeholder="0"
+          />
+        </Grid>
         <Grid size={12}>
           <ConductorInputNumber
             id="task-pollTimeoutSeconds-field"

ui-next/src/pages/definition/task/state/helpers.test.ts

@@ -0,0 +1,58 @@
+/**
+ * Save used to be disabled whenever the description was empty, with nothing on
+ * screen saying so — and the rule was not even applied consistently (new task
+ * definitions bypassed it, so a definition could be created without a
+ * description and then never edited again). The API marks description optional;
+ * only ownerEmail is required. These tests pin what may and may not block Save.
+ */
+import { describe, expect, it } from "vitest";
+import { isSaveDisabled } from "./helpers";
+
+const editingExisting = {
+  noChanges: false,
+  isNewTaskDef: false,
+  isTrialExpired: false,
+};
+
+describe("isSaveDisabled", () => {
+  it("allows saving an edited definition", () => {
+    expect(isSaveDisabled(editingExisting)).toBe(false);
+  });
+
+  it("blocks saving an unchanged existing definition", () => {
+    expect(isSaveDisabled({ ...editingExisting, noChanges: true })).toBe(true);
+  });
+
+  it("allows saving a brand new definition that has no changes yet", () => {
+    expect(
+      isSaveDisabled({
+        ...editingExisting,
+        isNewTaskDef: true,
+        noChanges: true,
+      }),
+    ).toBe(false);
+  });
+
+  it("blocks saving when the JSON cannot be parsed", () => {
+    expect(isSaveDisabled({ ...editingExisting, jsonInvalid: true })).toBe(
+      true,
+    );
+  });
+
+  it("blocks saving on an expired trial", () => {
+    expect(isSaveDisabled({ ...editingExisting, isTrialExpired: true })).toBe(
+      true,
+    );
+  });
+
+  it("treats an undefined isNewTaskDef as an existing definition", () => {
+    // The form machine's context does not carry the flag.
+    expect(
+      isSaveDisabled({
+        noChanges: true,
+        isTrialExpired: false,
+        isNewTaskDef: undefined,
+      }),
+    ).toBe(true);
+  });
+});

ui-next/src/pages/definition/task/state/helpers.ts

@@ -65,3 +65,24 @@ export const parseErrors = (errors: ErrorObject[] | null) =>
         {},
       )
     : {};
+
+/**
+ * Save is blocked only by conditions that would make the request fail or be a
+ * no-op. An empty description is deliberately NOT one of them: the API marks
+ * description optional (only ownerEmail is required), and gating Save on it
+ * left the button dead with nothing on screen explaining why.
+ *
+ * isNewTaskDef is optional because the form machine's context does not carry
+ * it, so it arrives undefined there.
+ */
+export const isSaveDisabled = ({
+  noChanges,
+  isNewTaskDef,
+  isTrialExpired,
+  jsonInvalid = false,
+}: {
+  noChanges: boolean;
+  isNewTaskDef?: boolean;
+  isTrialExpired: boolean;
+  jsonInvalid?: boolean;
+}) => jsonInvalid || (!isNewTaskDef && noChanges) || isTrialExpired;