Skip to content

fix(dashboard): validate manifest mode fields #6914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
houko merged 4 commits into from
Aug 12, 2026
Merged

fix(dashboard): validate manifest mode fields #6914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb62bd2
fix(dashboard): validate manifest mode fields
houko Aug 10, 2026
acf33fc
docs: fix changelog fragment format for #6914
houko Aug 10, 2026
5cce269
fix(dashboard): clean up compact manifest controls
houko Aug 10, 2026
30215b4
docs(dashboard/agents): rewrap two new comments to one sentence per line
claude Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/fixed/6914-agent-manifest-validation-followups.md
View file
Open in desktop

@houko houko Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking nit, not covered by the earlier review on this PR: commit acf33fcdc78ea88eb7cbfa4704a073e5a39cbbdd ("docs: fix changelog fragment format for #6914") has a hard-wrapped message body ("...fragments sort\nusefully, unwrap the hard-wrapped body..."), breaking mid-sentence rather than at sentence boundaries. CLAUDE.md's prose-wrapping rule covers commit message bodies too (only the subject line gets the git-display-truncation exception). Not asking for a history rewrite here — flagging only, since the fix would need an amend + force-push on an already-open PR.

Everything else looks solid: the changelog fragment itself is correctly named/sectioned/formatted, the validation logic (parseSupportedJsonSchema, hasUnsupportedJsonNumber) correctly detects precision-losing numbers by scanning the raw JSON text rather than the already-rounded parsed value, and the TagInput fix now clears the input on a duplicate submission instead of only on a successful add.

Generated by Claude Code

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The dashboard agent editor now blocks periodic schedules without a cron expression and JSON-schema response formats whose schemas are empty, malformed, or cannot be represented faithfully in TOML.
Validation errors automatically open their sections and are exposed to assistive technology.
It also removes a redundant schedule parsing branch, clears duplicate tag submissions, and gives the stream-thinking toggle an accessible name.
(#6914) (@houko)
78 changes: 76 additions & 2 deletions crates/librefang-api/dashboard/src/components/AgentManifestForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,23 @@ function Harness({
skillCatalog,
toolCatalog,
mcpCatalog,
initialState,
invalidFields = new Set(),
}: {
skillCatalog?: ManifestCatalogEntry[];
toolCatalog?: ManifestCatalogEntry[];
mcpCatalog?: ManifestCatalogEntry[];
initialState?: ManifestFormState;
invalidFields?: Set<string>;
}) {
const [state, setState] = useState<ManifestFormState>(() => emptyManifestForm());
const [state, setState] = useState<ManifestFormState>(() => initialState ?? emptyManifestForm());
return (
<AgentManifestForm
value={state}
onChange={setState}
providers={[{ name: "openai" }]}
models={[{ provider: "openai", id: "gpt-4o" }]}
invalidFields={new Set()}
invalidFields={invalidFields}
extras={emptyManifestExtras()}
skillCatalog={skillCatalog}
toolCatalog={toolCatalog}
Expand All @@ -45,6 +49,47 @@ function Harness({
);
}

describe("AgentManifestForm — validation feedback", () => {
it("opens scheduling errors and exposes the cron error to assistive technology", () => {
const state = emptyManifestForm();
state.schedule = { mode: "periodic", cron: "" };

render(<Harness initialState={state} invalidFields={new Set(["schedule.cron"])} />);

const input = screen.getByRole("textbox", { name: "agents.form.cron" });
expect(input).toHaveAttribute("aria-invalid", "true");
expect(input).toHaveAttribute("aria-required", "true");
expect(input).toHaveAccessibleDescription("agents.form.cron_required_error");
expect(input.closest("details")).toHaveAttribute("open");
expect(input.closest("details")?.querySelector("summary")).toHaveAttribute(
"aria-invalid",
"true",
);
});

it("opens response-format errors and exposes the schema error to assistive technology", () => {
const state = emptyManifestForm();
state.response_format = { mode: "json_schema", name: "response", schema: "", strict: false };

render(
<Harness
initialState={state}
invalidFields={new Set(["response_format.schema"])}
/>,
);

const textarea = screen.getByRole("textbox", { name: "agents.form.schema_body" });
expect(textarea).toHaveAttribute("aria-invalid", "true");
expect(textarea).toHaveAttribute("aria-required", "true");
expect(textarea).toHaveAccessibleDescription("agents.form.schema_invalid_error");
expect(textarea.closest("details")).toHaveAttribute("open");
expect(textarea.closest("details")?.querySelector("summary")).toHaveAttribute(
"aria-invalid",
"true",
);
});
});

describe("AgentManifestForm — tools/skills/mcp selection (#5246)", () => {
it("clicking a tool option from the dropdown adds it as a chip", async () => {
const user = userEvent.setup();
Expand Down Expand Up @@ -141,3 +186,32 @@ describe("AgentManifestForm — tools/skills/mcp selection (#5246)", () => {
expect(within(list).getByText("write_file")).toBeInTheDocument();
});
});

describe("AgentManifestForm — compact controls", () => {
it("clears duplicate text submitted to a tag input", async () => {
const user = userEvent.setup();
const state = emptyManifestForm();
state.mcp_servers = ["filesystem"];
render(<Harness initialState={state} />);

const removeButton = screen.getByRole("button", { name: "remove filesystem" });
const input = removeButton.parentElement?.parentElement?.querySelector("input");
expect(input).toBeInstanceOf(HTMLInputElement);
if (!(input instanceof HTMLInputElement)) return;

await user.type(input, "filesystem{Enter}");
expect(input).toHaveValue("");
expect(screen.getAllByRole("button", { name: "remove filesystem" })).toHaveLength(1);
});

it("gives the stream-thinking checkbox an accessible name", async () => {
const user = userEvent.setup();
render(<Harness />);

await user.click(screen.getByRole("checkbox", { name: "agents.form.thinking_enabled" }));

expect(
screen.getByRole("checkbox", { name: "agents.form.stream_thinking" }),
).toBeInTheDocument();
});
});
79 changes: 71 additions & 8 deletions crates/librefang-api/dashboard/src/components/AgentManifestForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,11 @@ export function AgentManifestForm({
</Field>
</Section>

<CollapsibleSection title={t("agents.form.scheduling")} defaultOpen={false}>
<CollapsibleSection
title={t("agents.form.scheduling")}
defaultOpen={false}
invalid={invalidFields.has("schedule.cron")}
>
<Field label={t("agents.form.schedule_mode")} hint={t("agents.form.schedule_mode_hint")}>
<select
value={value.schedule.mode}
Expand All @@ -513,13 +517,29 @@ export function AgentManifestForm({
</select>
</Field>
{value.schedule.mode === "periodic" && (
<Field label={t("agents.form.cron")} hint={t("agents.form.cron_hint")}>
<Field
label={t("agents.form.cron")}
hint={t("agents.form.cron_hint")}
required
invalid={invalidFields.has("schedule.cron")}
error={t("agents.form.cron_required_error")}
errorId="agent-manifest-schedule-cron-error"
>
<input
id="agent-manifest-schedule-cron"
type="text"
value={value.schedule.cron}
onChange={(e) => update({ schedule: { mode: "periodic", cron: e.target.value } })}
placeholder={t("agents.form.cron_placeholder")}
className={inputClass}
aria-label={t("agents.form.cron")}
aria-invalid={invalidFields.has("schedule.cron") || undefined}
aria-required="true"
aria-describedby={
invalidFields.has("schedule.cron")
? "agent-manifest-schedule-cron-error"
: undefined
}
/>
</Field>
)}
Expand Down Expand Up @@ -638,6 +658,7 @@ export function AgentManifestForm({
<Field label={t("agents.form.stream_thinking")}>
<Toggle
label=""
ariaLabel={t("agents.form.stream_thinking")}
checked={value.thinking.stream_thinking}
onChange={(checked) => updateThinking({ stream_thinking: checked })}
/>
Expand Down Expand Up @@ -862,7 +883,11 @@ export function AgentManifestForm({
</button>
</CollapsibleSection>

<CollapsibleSection title={t("agents.form.response_format")} defaultOpen={false}>
<CollapsibleSection
title={t("agents.form.response_format")}
defaultOpen={false}
invalid={invalidFields.has("response_format.schema")}
>
{value.response_format.mode === "text" && extras.topLevel.response_format !== undefined && (
<ExtrasOverrideHint message={t("agents.form.response_format_extras_hint")} />
)}
Expand Down Expand Up @@ -902,8 +927,15 @@ export function AgentManifestForm({
className={inputClass}
/>
</Field>
<Field label={t("agents.form.schema_body")}>
<Field
label={t("agents.form.schema_body")}
required
invalid={invalidFields.has("response_format.schema")}
error={t("agents.form.schema_invalid_error")}
errorId="agent-manifest-response-schema-error"
>
<textarea
id="agent-manifest-response-schema"
value={jsonSchemaFormat.schema}
onChange={(e) =>
update({
Expand All @@ -917,6 +949,14 @@ export function AgentManifestForm({
}
rows={6}
className={textareaClass}
aria-label={t("agents.form.schema_body")}
aria-invalid={invalidFields.has("response_format.schema") || undefined}
aria-required="true"
aria-describedby={
invalidFields.has("response_format.schema")
? "agent-manifest-response-schema-error"
: undefined
}
/>
</Field>
<Toggle
Expand Down Expand Up @@ -1103,20 +1143,27 @@ function CollapsibleSection({
title,
children,
defaultOpen,
invalid,
}: {
title: string;
children: React.ReactNode;
defaultOpen?: boolean;
invalid?: boolean;
}) {
return (
<details
className="group rounded-xl border border-border-subtle/60 bg-surface/40 overflow-hidden"
open={defaultOpen}
open={defaultOpen || invalid}
>
<summary
aria-invalid={invalid || undefined}
className="flex items-center justify-between p-3 cursor-pointer list-none select-none"
>
<span className="text-[10px] font-bold uppercase tracking-widest text-text-dim">
<span
className={`text-[10px] font-bold uppercase tracking-widest ${
invalid ? "text-error" : "text-text-dim"
}`}
>
{title}
</span>
<ChevronDown className="w-4 h-4 text-text-dim transition-transform group-open:rotate-180" />
Expand All @@ -1131,12 +1178,16 @@ function Field({
hint,
required,
invalid,
error,
errorId,
children,
}: {
label: string;
hint?: string;
required?: boolean;
invalid?: boolean;
error?: string;
errorId?: string;
children: React.ReactNode;
}) {
// Use a <div> wrapper rather than a <label> (#5246). A <label>
Expand Down Expand Up @@ -1164,6 +1215,15 @@ function Field({
</span>
)}
<span className={label ? "mt-1 block" : "block"}>{children}</span>
{invalid && error && (
<span
id={errorId}
className="mt-1 block text-[10px] text-error"
role="alert"
>
{error}
</span>
)}
{hint && <span className="mt-1 text-[10px] text-text-dim/70 block">{hint}</span>}
</div>
);
Expand All @@ -1180,10 +1240,12 @@ function ExtrasOverrideHint({ message }: { message: string }) {

function Toggle({
label,
ariaLabel,
checked,
onChange,
}: {
label: string;
ariaLabel?: string;
checked: boolean;
onChange: (next: boolean) => void;
}) {
Expand All @@ -1192,10 +1254,11 @@ function Toggle({
<input
type="checkbox"
checked={checked}
aria-label={ariaLabel}
onChange={(e) => onChange(e.target.checked)}
className="h-4 w-4 rounded border-border-subtle accent-brand"
/>
{label}
{label ? <span>{label}</span> : null}
</label>
);
}
Expand All @@ -1214,9 +1277,9 @@ function TagInput({
const commit = (raw: string): void => {
const cleaned = raw.trim();
if (!cleaned) return;
setInputValue("");
if (value.includes(cleaned)) return;
onChange([...value, cleaned]);
setInputValue("");
};
return (
<div className="flex flex-wrap items-center gap-1.5 rounded-lg border border-border-subtle bg-main px-2 py-1.5 focus-within:border-brand">
Expand Down
Loading