MUL-5587: fix(autopilots): tell the user which required field blocks Create (#6231) (#6237)

* fix(autopilots): tell the user which required field blocks Create (MUL-5587)

The create dialog gated its submit button with native `disabled` on an
empty title, an unpicked assignee, or a server-rejected schedule. A
natively disabled button is neither hoverable nor focusable, so a user
who had not picked an assignee got a dead grey control and no hint about
what was missing (GitHub #6231) — the assignee picker looks exactly like
the optional Project and Subscribers pickers beside it, and nothing said
only that one was required.

An unmet requirement now uses `aria-disabled` instead: the button still
takes the hover that shows a tooltip naming the missing field, and still
takes the click, which reveals an inline error under the field at fault
and focuses it. `handleSubmit` is the real gate either way, so nothing
can be submitted that could not be before. The assignee section also
carries a required marker up front, so the dead end is avoided rather
than only explained.

The three states come from one `submitBlock` value shared by the button,
the tooltip, the inline errors and `handleSubmit`, so a rendered
affordance cannot disagree with what submitting actually does.

Co-authored-by: multica-agent <github@multica.ai>

* refactor(autopilots): stop dimming Create at all — the click is the feedback (MUL-5587)

Follow-up on review. The previous commit kept the button greyed via
`aria-disabled` and explained the grey with a tooltip. Since a blocked
click now intercepts and points at the offending field, the greying earns
nothing: drop `aria-disabled`, the dimming classes and the tooltip, and
let the button be an ordinary live button whenever a save isn't already
in flight.

The schedule case no longer scrolls to the editor's inline error and
returns. It falls through to `scheduleGate.ensureAccepted`, which re-asks
the server and toasts its actual reason — visible feedback where the
scroll could be a no-op, and it self-heals when a stored expression the
server once rejected is accepted again.

That leaves nothing in this dialog reading the gate's `scheduleValid`, so
its `onValidityChange` / `clearRejection` wiring goes too rather than
sitting inert. The shared hook keeps them for the detail page's
add-trigger dialog, which still gates on it.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-08-01 11:04:10 +08:00
committed by GitHub
parent 8df7549d84
commit 37f3bb7dd9
6 changed files with 290 additions and 18 deletions

View File

@@ -1,6 +1,6 @@
"use client";
import { useMemo, useRef, useState } from "react";
import { useId, useMemo, useRef, useState } from "react";
import { useQuery } from "@tanstack/react-query";
import { toast } from "sonner";
import {
@@ -53,7 +53,7 @@ import type {
AutopilotExecutionMode,
AutopilotTrigger,
} from "@multica/core/types";
import { TitleEditor, ContentEditor } from "../../editor";
import { TitleEditor, ContentEditor, type TitleEditorRef } from "../../editor";
import { ActorAvatar } from "../../common/actor-avatar";
import { SegmentedToggle } from "../../common/segmented-toggle";
import { ProjectPicker } from "../../projects/components/project-picker";
@@ -251,14 +251,32 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
// edits to the title, prompt or assignee.
const scheduleWillBeWritten =
triggerKind === "schedule" && !schedulePillDisabled && (isCreate || scheduleDirty);
const canSubmit =
title.trim().length > 0 &&
assigneeId.length > 0 &&
!submitting &&
(!scheduleWillBeWritten || scheduleGate.scheduleValid);
// The FIRST empty required field in reading order — the user fills one, the
// next surfaces. Only these two are answered here: a rejected schedule is
// re-checked against the server below, which toasts its actual reason.
const missingField: "title" | "assignee" | null =
title.trim().length === 0 ? "title" : assigneeId.length === 0 ? "assignee" : null;
// Inline errors appear only after a submit attempt: a form that opens already
// shouting at the user for fields they have not reached yet is worse than the
// silence this replaces. Rendered as `showErrors && <field is still empty>`,
// so filling the field clears its error without a second submit.
const [showErrors, setShowErrors] = useState(false);
const titleEditorRef = useRef<TitleEditorRef>(null);
const assigneeTriggerRef = useRef<HTMLButtonElement>(null);
const assigneeErrorId = useId();
const handleSubmit = async () => {
if (!canSubmit) return;
if (submitting) return;
if (missingField !== null) {
// Reveal the inline errors and take the user to the field at fault;
// focusing scrolls the config column to it on its own.
setShowErrors(true);
if (missingField === "title") titleEditorRef.current?.focus();
else assigneeTriggerRef.current?.focus();
return;
}
setSubmitting(true);
try {
if (scheduleWillBeWritten && !(await scheduleGate.ensureAccepted(schedule))) {
@@ -519,6 +537,7 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
<div className="flex-none lg:flex-1 min-h-0 flex flex-col border-b lg:border-b-0 lg:border-r">
<div className="px-6 pt-5 pb-3 shrink-0">
<TitleEditor
ref={titleEditorRef}
autoFocus={isCreate}
defaultValue={initial.title ?? ""}
placeholder={t(($) => $.dialog.title_placeholder)}
@@ -526,6 +545,15 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
onChange={setTitle}
onSubmit={handleSubmit}
/>
{/* role="alert": the title is a contenteditable, so there is no
input to hang aria-describedby off — announcing the error is
the only way a screen-reader user learns why Create did
nothing. */}
{showErrors && title.trim().length === 0 && (
<p role="alert" className="mt-1.5 text-caption text-destructive">
{t(($) => $.dialog.error_title_required)}
</p>
)}
</div>
<div className="px-6 pb-2 shrink-0 flex items-baseline gap-2">
@@ -553,11 +581,14 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
{/* Right: Configuration */}
<aside className="w-full lg:w-[380px] shrink-0 overflow-visible lg:overflow-y-auto px-5 py-5 space-y-5 bg-muted/30">
<AgentSection
ref={assigneeTriggerRef}
selectedType={assigneeType}
selectedId={assigneeId}
onChange={handleAssigneeChange}
selectedName={selectedAssignee?.name}
selectedDescription={selectedAssignee?.description}
invalid={showErrors && assigneeId.length === 0}
errorId={assigneeErrorId}
/>
<OutputModeSection mode={executionMode} onChange={setExecutionMode} />
@@ -584,14 +615,16 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
{triggerKind === "schedule" ? (
<div>
<SectionLabel>{t(($) => $.dialog.section_schedule)}</SectionLabel>
{/* No `onValidityChange` / `clearRejection` here, unlike the
detail page's add-trigger dialog: nothing in this footer is
gated on the schedule's validity any more, so the gate's
`scheduleValid` would have no reader. The editor still shows
its own inline rejection, and `ensureAccepted` re-asks the
server on submit and toasts what it says. */}
<ScheduleEditor
value={schedule}
onChange={(next) => {
scheduleGate.clearRejection();
setSchedule(next);
}}
onChange={setSchedule}
wsId={wsId}
onValidityChange={scheduleGate.onValidityChange}
// Locked while the save is in flight: the submit path validates
// over the network and then writes the schedule it read before
// that round trip, so an edit made in between would be dropped
@@ -624,14 +657,24 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
<Button size="sm" variant="outline" onClick={() => onOpenChange(false)}>
{t(($) => $.dialog.cancel)}
</Button>
<Button size="sm" onClick={handleSubmit} disabled={!canSubmit}>
{/* Live whenever a save isn't already in flight — an unmet
requirement never dims it. A greyed-out button is a dead end
with no room for a reason (#6231); a live one answers the click
with an inline error on the field at fault, which says more than
any disabled state could. `handleSubmit` is the gate. */}
<Button
size="sm"
onClick={handleSubmit}
disabled={submitting}
aria-busy={submitting || undefined}
>
{submitting
? isCreate
? t(($) => $.dialog.creating)
: t(($) => $.dialog.saving)
: isCreate
? t(($) => $.dialog.create)
: t(($) => $.dialog.save)}
? t(($) => $.dialog.create)
: t(($) => $.dialog.save)}
</Button>
</div>
</div>
@@ -646,42 +689,71 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
// Right column sections
// ---------------------------------------------------------------------------
function SectionLabel({ children }: { children: React.ReactNode }) {
function SectionLabel({
children,
required,
}: {
children: React.ReactNode;
// Purely the sighted user's advance warning. `aria-required` is not supported
// on `role="button"`, so the picker cannot carry it; what a screen reader
// gets instead is the blocked submit's error, wired to the trigger through
// aria-describedby and announced by its own role="alert".
required?: boolean;
}) {
return (
<div className="text-micro font-semibold tracking-[0.08em] text-muted-foreground uppercase mb-2">
{children}
{required === true && (
<span aria-hidden className="ml-0.5 text-destructive">
*
</span>
)}
</div>
);
}
function AgentSection({
ref,
selectedType,
selectedId,
onChange,
selectedName,
selectedDescription,
invalid,
errorId,
}: {
ref: React.Ref<HTMLButtonElement>;
selectedType: AutopilotAssigneeType;
selectedId: string;
onChange: (next: AssigneeSelection) => void;
selectedName?: string;
selectedDescription?: string;
/** A submit was attempted with no assignee picked. */
invalid: boolean;
errorId: string;
}) {
const { t } = useT("autopilots");
const hasSelection = selectedId.length > 0;
return (
<div>
<SectionLabel>{t(($) => $.dialog.section_assignee)}</SectionLabel>
{/* Marked required, unlike the Project and Subscribers pickers below it:
the three look identical, and nothing else told the user that only
this one blocks Create (#6231). */}
<SectionLabel required>{t(($) => $.dialog.section_assignee)}</SectionLabel>
<AgentPicker
assignee={hasSelection ? { type: selectedType, id: selectedId } : null}
onChange={onChange}
align="start"
triggerRender={
<button
ref={ref}
type="button"
aria-invalid={invalid || undefined}
aria-describedby={invalid ? errorId : undefined}
className={cn(
"w-full flex items-center gap-2.5 rounded-md border bg-background px-3 py-2 text-left",
"hover:bg-accent/40 transition-colors cursor-pointer",
invalid && "border-destructive",
)}
>
{hasSelection ? (
@@ -710,6 +782,11 @@ function AgentSection({
</button>
}
/>
{invalid && (
<p id={errorId} role="alert" className="mt-1.5 text-caption text-destructive">
{t(($) => $.dialog.error_assignee_required)}
</p>
)}
</div>
);
}

View File

@@ -0,0 +1,187 @@
import { useImperativeHandle, useRef, useState } from "react";
import { describe, it, expect, vi, beforeEach } from "vitest";
import { screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { renderWithI18n } from "../../test/i18n";
// Regression cover for GitHub #6231: "Create Autopilot" was disabled whenever
// a required field was empty, so a user who had not picked an assignee saw a
// dead control and no reason for it. The button is now live whenever a save
// isn't already in flight, and the click it accepts is what surfaces an inline
// error on the field at fault.
const mockCreateAutopilot = vi.hoisted(() => vi.fn());
const mockCreateTrigger = vi.hoisted(() => vi.fn());
vi.mock("@multica/core/hooks", () => ({ useWorkspaceId: () => "ws-test" }));
vi.mock("@multica/core/paths", () => ({ useCurrentWorkspace: () => ({ name: "Acme" }) }));
vi.mock("@multica/core/workspace/queries", () => ({
agentListOptions: (wsId: string) => ({
queryKey: ["agents", wsId],
queryFn: async () => [
{ id: "agent-1", name: "Scout", description: "Researches things", archived_at: null },
],
}),
squadListOptions: (wsId: string) => ({
queryKey: ["squads", wsId],
queryFn: async () => [],
}),
}));
vi.mock("@multica/core/projects/queries", () => ({
projectListOptions: (wsId: string) => ({
queryKey: ["projects", wsId],
queryFn: async () => [],
}),
}));
vi.mock("@multica/core/autopilots/queries", () => ({
cronPreviewOptions: (wsId: string, expr: string, tz: string) => ({
queryKey: ["cron-preview", wsId, expr, tz],
queryFn: async () => ({ next_runs: ["2126-07-14T01:00:00Z"] }),
retry: false,
}),
}));
vi.mock("@multica/core/autopilots/mutations", () => ({
useCreateAutopilot: () => ({ mutateAsync: mockCreateAutopilot }),
useCreateAutopilotTrigger: () => ({ mutateAsync: mockCreateTrigger }),
useUpdateAutopilot: () => ({ mutateAsync: vi.fn() }),
useUpdateAutopilotTrigger: () => ({ mutateAsync: vi.fn() }),
}));
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
// Tiptap in jsdom is neither cheap nor the subject here: the title is a plain
// input whose ref honours focus(), which is all the dialog asks of it.
vi.mock("../../editor", () => ({
TitleEditor: ({ ref, defaultValue, placeholder, onChange, onSubmit }: any) => {
const [value, setValue] = useState(defaultValue ?? "");
const inputRef = useRef<HTMLInputElement>(null);
useImperativeHandle(ref, () => ({
getText: () => value,
focus: () => inputRef.current?.focus(),
focusAtCoords: () => inputRef.current?.focus(),
}));
return (
<input
ref={inputRef}
aria-label="title"
value={value}
placeholder={placeholder}
onChange={(e) => {
setValue(e.target.value);
onChange?.(e.target.value);
}}
onKeyDown={(e) => {
if (e.key === "Enter") onSubmit?.();
}}
/>
);
},
ContentEditor: ({ placeholder }: any) => <textarea aria-label="runbook" placeholder={placeholder} />,
}));
vi.mock("../../common/actor-avatar", () => ({
ActorAvatar: ({ actorId }: { actorId: string }) => <span data-testid="actor-avatar">{actorId}</span>,
}));
vi.mock("./subscriber-multi-select", () => ({
SubscriberMultiSelect: () => <div data-testid="subscriber-multi-select" />,
}));
vi.mock("../../projects/components/project-picker", () => ({
ProjectPicker: ({ triggerRender }: { triggerRender: React.ReactElement }) => triggerRender,
}));
vi.mock("../pickers/timezone-picker", () => ({
TimezonePicker: ({ value }: { value: string }) => <div data-testid="timezone-picker">{value}</div>,
}));
import { AutopilotDialog } from "./autopilot-dialog";
function renderCreateDialog() {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
return renderWithI18n(
<QueryClientProvider client={qc}>
<AutopilotDialog mode="create" open onOpenChange={vi.fn()} />
</QueryClientProvider>,
);
}
const createButton = () => screen.getByRole("button", { name: "Create autopilot" });
const assigneeTrigger = () => screen.getByRole("button", { name: /Select agent or squad/ });
describe("AutopilotDialog required-field feedback", () => {
beforeEach(() => {
mockCreateAutopilot.mockReset();
mockCreateTrigger.mockReset();
});
it("leaves the create button fully live while required fields are empty", () => {
renderCreateDialog();
// A dimmed button is a dead end with no room for a reason. Neither the
// native attribute nor the ARIA one may be set: the click is the whole
// feedback channel.
expect(createButton()).not.toBeDisabled();
expect(createButton()).not.toHaveAttribute("aria-disabled");
});
it("names the missing title on a blocked submit instead of doing nothing", async () => {
const user = userEvent.setup();
renderCreateDialog();
expect(screen.queryByText("Enter a name for this autopilot.")).not.toBeInTheDocument();
await user.click(createButton());
expect(await screen.findByText("Enter a name for this autopilot.")).toBeInTheDocument();
expect(mockCreateAutopilot).not.toHaveBeenCalled();
});
it("names the missing assignee once the title is filled, and marks the picker invalid", async () => {
const user = userEvent.setup();
renderCreateDialog();
await user.type(screen.getByLabelText("title"), "Daily digest");
await user.click(createButton());
expect(
await screen.findByText("Choose the agent or squad that will run this autopilot."),
).toBeInTheDocument();
// The title error clears itself the moment the field is filled — no second
// submit needed to retire an error the user has already fixed.
expect(screen.queryByText("Enter a name for this autopilot.")).not.toBeInTheDocument();
expect(assigneeTrigger()).toHaveAttribute("aria-invalid", "true");
expect(mockCreateAutopilot).not.toHaveBeenCalled();
});
it("clears the assignee error and submits once an agent is picked", async () => {
const user = userEvent.setup();
mockCreateAutopilot.mockResolvedValue({ id: "ap-1" });
mockCreateTrigger.mockResolvedValue({ id: "tr-1" });
renderCreateDialog();
await user.type(screen.getByLabelText("title"), "Daily digest");
await user.click(createButton());
await screen.findByText("Choose the agent or squad that will run this autopilot.");
await user.click(assigneeTrigger());
await user.click(await screen.findByRole("button", { name: /Scout/ }));
await waitFor(() => {
expect(
screen.queryByText("Choose the agent or squad that will run this autopilot."),
).not.toBeInTheDocument();
});
await user.click(createButton());
await waitFor(() => expect(mockCreateAutopilot).toHaveBeenCalledTimes(1));
expect(mockCreateAutopilot.mock.calls[0]?.[0]).toMatchObject({
title: "Daily digest",
assignee_type: "agent",
assignee_id: "agent-1",
});
});
});

View File

@@ -306,6 +306,8 @@
"event_filter_actions_placeholder": "completed, failed",
"event_filter_hint": "Only process webhooks matching these events. Leave empty to accept all.",
"schedule_disabled_reason": "This autopilot has multiple schedules — edit them in the detail page.",
"error_title_required": "Enter a name for this autopilot.",
"error_assignee_required": "Choose the agent or squad that will run this autopilot.",
"output_modes": {
"create_issue": {
"label": "Create issue",

View File

@@ -306,6 +306,8 @@
"event_filter_actions_placeholder": "completed, failed",
"event_filter_hint": "これらのイベントに一致する Webhook のみを処理します。空にするとすべて許可します。",
"schedule_disabled_reason": "このオートパイロットには複数のスケジュールがあります。詳細ページで編集してください。",
"error_title_required": "オートパイロット名を入力してください。",
"error_assignee_required": "実行するエージェントまたはスクワッドを選択してください。",
"output_modes": {
"create_issue": {
"label": "イシューを作成",

View File

@@ -306,6 +306,8 @@
"event_filter_actions_placeholder": "completed, failed",
"event_filter_hint": "이 이벤트와 일치하는 Webhook만 처리합니다. 비워 두면 모두 허용합니다.",
"schedule_disabled_reason": "이 오토파일럿에는 여러 일정이 있습니다. 상세 페이지에서 수정하세요.",
"error_title_required": "오토파일럿 이름을 입력하세요.",
"error_assignee_required": "실행할 에이전트 또는 스쿼드를 선택하세요.",
"output_modes": {
"create_issue": {
"label": "이슈 생성",

View File

@@ -306,6 +306,8 @@
"event_filter_actions_placeholder": "completed, failed",
"event_filter_hint": "只处理匹配这些事件的 webhook。留空则接受所有事件。",
"schedule_disabled_reason": "该自动化有多个时间表——请到详情页编辑。",
"error_title_required": "请填写自动化名称。",
"error_assignee_required": "请选择运行该自动化的智能体或小队。",
"output_modes": {
"create_issue": {
"label": "创建 issue",