fix: close workflow console review remediation

This commit is contained in:
lda
2026-08-12 14:51:02 +07:00 Verified
parent e0e1b718b8
commit a9bb2ab837
22 changed files with 359 additions and 178 deletions
@@ -23,7 +23,7 @@ describe("CapabilityNodeForm", () => {
await user.type(screen.getByRole("textbox", { name: "Step id" }), "enrich");
await user.type(screen.getByRole("textbox", { name: "Description" }), "Enrich report");
await user.type(screen.getByRole("textbox", { name: "Title" }), "Quarterly report");
expect(screen.getByRole("spinbutton", { name: "Timeout seconds" })).toHaveAttribute("inputmode", "decimal");
expect(screen.getByRole("spinbutton", { name: "Timeout seconds" })).toHaveAttribute("inputmode", "numeric");
await user.click(screen.getByRole("button", { name: "Add node" }));
expect(submissions[0]).toMatchObject({
@@ -56,6 +56,30 @@ describe("CapabilityNodeForm", () => {
expect(submissions[0]).not.toHaveProperty("description");
});
it("requires timeout seconds to be a positive whole number", async () => {
const user = userEvent.setup();
const submissions: unknown[] = [];
render(
<CapabilityNodeForm
capabilityName="demo.enrich"
inputSchema={{ type: "object", properties: {} }}
onSubmit={(value) => { submissions.push(value); }}
/>,
);
await user.type(screen.getByRole("textbox", { name: "Step id" }), "enrich");
const timeout = screen.getByRole("spinbutton", { name: "Timeout seconds" });
expect(timeout).toHaveAttribute("min", "1");
expect(timeout).toHaveAttribute("step", "1");
await user.type(timeout, "1.5");
await user.click(screen.getByRole("button", { name: "Add node" }));
expect(screen.getByRole("alert")).toHaveTextContent(
"Timeout must be a whole number greater than 0.",
);
expect(submissions).toHaveLength(0);
});
it("reports local edits as dirty and keeps them when submission fails", async () => {
const user = userEvent.setup();
const dirtyStates: boolean[] = [];
@@ -134,8 +134,8 @@ export const CapabilityNodeForm = ({
const timeout = timeoutSecondsRef.current?.value.trim() ?? "";
if (timeout !== "") {
const value = Number(timeout);
if (!Number.isFinite(value) || value <= 0) {
issues.push("Timeout must be greater than 0.");
if (!Number.isFinite(value) || !Number.isInteger(value) || value <= 0) {
issues.push("Timeout must be a whole number greater than 0.");
}
}
return issues;
@@ -233,14 +233,14 @@ export const CapabilityNodeForm = ({
? ""
: String(initialValue.timeoutSeconds)
}
inputMode="decimal"
min="0.000001"
inputMode="numeric"
min={1}
ref={timeoutSecondsRef}
onChange={(event) => {
timeoutTouchedRef.current = true;
notifyMetadataChange();
}}
step="any"
step={1}
type="number"
/>
{metadataMessage("timeout_seconds") && <p role="alert">{metadataMessage("timeout_seconds")}</p>}
@@ -101,6 +101,15 @@ describe("SelectedCapabilityInspector", () => {
expect(screen.getAllByRole("tab")).toHaveLength(3);
expect(screen.getByRole("tab", { name: "Setup" })).toHaveAttribute("aria-selected", "true");
screen.getByRole("tab", { name: "Setup" }).focus();
await user.keyboard("{ArrowRight}");
expect(screen.getByRole("tab", { name: "Inputs" })).toHaveFocus();
expect(screen.getByRole("tab", { name: "Inputs" })).toHaveAttribute("aria-selected", "true");
await user.keyboard("{End}");
expect(screen.getByRole("tab", { name: "Outputs" })).toHaveFocus();
await user.keyboard("{Home}");
expect(screen.getByRole("tab", { name: "Setup" })).toHaveFocus();
await user.click(screen.getByRole("tab", { name: "Inputs" }));
expect(screen.getByRole("region", { name: "Raw unsupported input row 2" })).toHaveTextContent("broken");
await user.click(screen.getByRole("button", { name: "Save inputs" }));
@@ -1,4 +1,4 @@
import { useState, type ReactNode } from "react";
import { useState, type KeyboardEvent, type ReactNode } from "react";
import type { CapabilityDetail } from "../domain/capability-models.js";
import type { DraftDiagnostic, DraftWorkspace } from "../domain/draft-workspace-models.js";
import { CapabilitySetupForm } from "./CapabilitySetupForm.js";
@@ -64,6 +64,7 @@ const setupDiagnostics = (
const emptyStateSchema = { type: "object", properties: {} };
const tabLabels: Record<InspectorTab, string> = { setup: "Setup", inputs: "Inputs", outputs: "Outputs" };
const inspectorTabs = Object.keys(tabLabels) as InspectorTab[];
const tabPanelId = (tab: InspectorTab): string => `selected-step-panel-${tab}`;
const tabId = (tab: InspectorTab): string => `selected-step-tab-${tab}`;
@@ -98,6 +99,26 @@ export const SelectedCapabilityInspector = ({
capabilityDetailMessage,
}: SelectedCapabilityInspectorProps) => {
const [activeTab, setActiveTab] = useState<InspectorTab>("setup");
const activateTab = (tab: InspectorTab): void => {
setActiveTab(tab);
document.getElementById(tabId(tab))?.focus();
};
const handleTabKeyDown = (event: KeyboardEvent<HTMLButtonElement>, tab: InspectorTab): void => {
const currentIndex = inspectorTabs.indexOf(tab);
const targetIndex = event.key === "Home"
? 0
: event.key === "End"
? inspectorTabs.length - 1
: event.key === "ArrowRight"
? (currentIndex + 1) % inspectorTabs.length
: event.key === "ArrowLeft"
? (currentIndex - 1 + inspectorTabs.length) % inspectorTabs.length
: null;
if (targetIndex === null) return;
event.preventDefault();
const target = inspectorTabs[targetIndex];
if (target !== undefined) activateTab(target);
};
const rawStep = selectedStep(draft, stepId);
const projected = projectSelectedStepDataflow(draft, stepId);
const preservedForm = controller.preservedCapabilityForm?.kind === "update" &&
@@ -140,7 +161,7 @@ export const SelectedCapabilityInspector = ({
{!isUnsupported && (
<>
<div aria-label="Selected step views" className="selected-capability-inspector__tabs" role="tablist">
{(Object.keys(tabLabels) as InspectorTab[]).map((tab) => (
{inspectorTabs.map((tab) => (
<button
aria-controls={tabPanelId(tab)}
aria-selected={activeTab === tab}
@@ -148,6 +169,7 @@ export const SelectedCapabilityInspector = ({
id={tabId(tab)}
key={tab}
onClick={() => setActiveTab(tab)}
onKeyDown={(event) => handleTabKeyDown(event, tab)}
role="tab"
tabIndex={activeTab === tab ? 0 : -1}
type="button"
@@ -40,4 +40,19 @@ describe("canonical capability form projection", () => {
timeoutSeconds: null,
});
});
it("ignores negative and sparse array targets instead of creating array properties", () => {
const projected = canonicalCapabilityFormData(
draft({
use: "wf.std.concat",
input: [
{ target: { root: "local", parts: ["items", "-1"] }, value: "negative" },
{ target: { root: "local", parts: ["items", "2"] }, value: "sparse" },
],
}),
"render",
);
expect(projected?.initialInputValue).toEqual({ items: [] });
});
});
@@ -52,7 +52,7 @@ const setPath = (current: unknown, path: string, value: unknown): unknown => {
}
if (Array.isArray(current)) {
const index = Number(head);
if (!Number.isInteger(index)) return current;
if (!Number.isInteger(index) || index < 0 || index > current.length) return current;
const next = [...current];
next[index] = setPath(next[index], formatTOMLPath(tail), value);
return next;
@@ -70,4 +70,11 @@ describe("withDiagnosticKeys", () => {
withDifferentHint,
]);
});
it("deduplicates equivalent nested details regardless of object key order", () => {
const left = diagnostic({ details: { field: "title", context: { row: 1, source: "input" } } });
const right = diagnostic({ details: { context: { source: "input", row: 1 }, field: "title" } });
expect(withDiagnosticKeys([left, right])).toHaveLength(1);
});
});
@@ -5,6 +5,19 @@ export type DiagnosticEntry = {
readonly key: string;
};
const isRecord = (value: unknown): value is Readonly<Record<string, unknown>> =>
typeof value === "object" && value !== null && !Array.isArray(value);
const canonicalize = (value: unknown): unknown => {
if (Array.isArray(value)) return value.map(canonicalize);
if (!isRecord(value)) return value;
return Object.fromEntries(
Object.entries(value)
.toSorted(([left], [right]) => left.localeCompare(right))
.map(([key, item]) => [key, canonicalize(item)]),
);
};
const diagnosticIdentity = (diagnostic: DraftDiagnostic): string =>
JSON.stringify([
diagnostic.code,
@@ -12,7 +25,7 @@ const diagnosticIdentity = (diagnostic: DraftDiagnostic): string =>
diagnostic.message,
diagnostic.stepId,
diagnostic.repairHint,
diagnostic.details,
canonicalize(diagnostic.details),
]);
export const withDiagnosticKeys = (
@@ -17,6 +17,7 @@ export type SchemaFieldControlProps = {
readonly onValueChange: (field: SchemaField, value: unknown) => void;
readonly onSourceChange: (field: SchemaField, source: FieldSource) => void;
readonly onArrayItemRemove: (field: SchemaField, index: number) => void;
readonly arrayItemKey?: ((field: SchemaField, index: number) => string) | undefined;
readonly sourceSuggestions?: ReadonlyArray<string>;
readonly showSourceControl?: boolean;
readonly idPrefix?: string;
@@ -216,6 +217,7 @@ const LeafControl = ({
};
export const SchemaFieldControl = ({
arrayItemKey,
field,
value,
sources,
@@ -246,8 +248,9 @@ export const SchemaFieldControl = ({
{field.children.map((child) => (
<SchemaFieldControl
diagnostics={diagnostics.filter((diagnostic) => isPathPrefix(child.path, diagnostic.path))}
arrayItemKey={arrayItemKey}
field={child}
key={pathKey(child)}
key={String(child.key)}
onArrayItemRemove={onArrayItemRemove}
onSourceChange={onSourceChange}
onValueChange={onValueChange}
@@ -282,8 +285,12 @@ export const SchemaFieldControl = ({
: null;
if (!itemField) return null;
return (
<div className="schema-form__array-item" key={pathKey(itemField)}>
<div
className="schema-form__array-item"
key={arrayItemKey?.(field, index) ?? pathKey(itemField)}
>
<SchemaFieldControl
arrayItemKey={arrayItemKey}
diagnostics={diagnostics.filter((diagnostic) => isPathPrefix(itemField.path, diagnostic.path))}
field={itemField}
onSourceChange={onSourceChange}
@@ -57,6 +57,42 @@ describe("SchemaForm", () => {
expect(screen.getByRole("button", { name: "Call capability" })).toBeInTheDocument();
});
it("isolates control ids and radio groups across form instances", () => {
render(
<>
<SchemaForm schema={{ type: "object", properties: { value: { type: "string" } } }} />
<SchemaForm schema={{ type: "object", properties: { value: { type: "string" } } }} />
</>,
);
const literalRadios = screen.getAllByRole("radio", { name: /Literal/ });
expect(literalRadios[0]).not.toHaveAttribute("name", literalRadios[1]?.getAttribute("name"));
const valueInputs = screen.getAllByRole("textbox", { name: "Value" });
expect(valueInputs[0]?.id).not.toBe(valueInputs[1]?.id);
});
it("resets values when the represented schema changes", async () => {
const user = userEvent.setup();
const { rerender } = render(
<SchemaForm
initialValue={{ first: "initial" }}
schema={{ type: "object", properties: { first: { type: "string" } } }}
/>,
);
await user.clear(screen.getByRole("textbox", { name: "First" }));
await user.type(screen.getByRole("textbox", { name: "First" }), "edited");
rerender(
<SchemaForm
initialValue={{ second: "replacement" }}
schema={{ type: "object", properties: { second: { type: "string" } } }}
/>,
);
expect(screen.queryByRole("textbox", { name: "First" })).not.toBeInTheDocument();
expect(screen.getByRole("textbox", { name: "Second" })).toHaveValue("replacement");
});
it("renders unsupported fields as JSON editors with their exact fallback reason", () => {
render(
<SchemaForm
@@ -152,6 +188,29 @@ describe("SchemaForm", () => {
]);
});
it("keeps later array controls mounted when an earlier row is removed", async () => {
const user = userEvent.setup();
render(
<SchemaForm
initialValue={{ items: [{ name: "first" }, { name: "second" }] }}
schema={{
type: "object",
properties: {
items: {
type: "array",
items: { type: "object", properties: { name: { type: "string" } } },
},
},
}}
/>,
);
const secondControl = screen.getAllByRole("textbox", { name: "Name" })[1];
await user.click(screen.getByRole("button", { name: "Remove item 1" }));
expect(screen.getByRole("textbox", { name: "Name" })).toBe(secondControl);
expect(secondControl).toHaveValue("second");
});
it("omits an untouched optional boolean but preserves explicit false", async () => {
const user = userEvent.setup();
const untouched: SchemaSerializationResult[] = [];
@@ -1,4 +1,4 @@
import { useState, type FormEvent, type ReactNode } from "react";
import { useId, useRef, useState, type FormEvent, type ReactNode } from "react";
import { normalizeSchema, type FieldSource, type SchemaField } from "./schema-field.js";
import { SchemaFieldControl } from "./SchemaFieldControl.js";
import {
@@ -93,7 +93,7 @@ const rawSchemaText = (schema: unknown): string => {
}
};
export const SchemaForm = ({
const SchemaFormState = ({
schema,
initialValue,
initialSources = EMPTY_SOURCES,
@@ -106,6 +106,9 @@ export const SchemaForm = ({
sourceSuggestions = EMPTY_SUGGESTIONS,
showSourceControls = true,
}: SchemaFormProps) => {
const formId = useId();
const nextArrayRowId = useRef(0);
const arrayRowIds = useRef(new Map<string, string[]>());
const field = normalizeSchema(schema);
const [values, setValues] = useState<unknown>(() =>
initialValue !== undefined ? initialValue : emptyValueFor(field),
@@ -147,6 +150,7 @@ export const SchemaForm = ({
onDirtyChange?.(true);
const arrayValue = readAtPath(values, arrayField.path);
if (!Array.isArray(arrayValue)) return;
arrayRowIds.current.get(sourceKey(arrayField))?.splice(index, 1);
const nextValues = setAtPath(
values,
arrayField.path,
@@ -160,6 +164,16 @@ export const SchemaForm = ({
onValueChange?.(serializeSchemaValues(field, nextValues, nextSources));
};
const arrayItemKey = (arrayField: SchemaField, index: number): string => {
const path = sourceKey(arrayField);
const ids = arrayRowIds.current.get(path) ?? [];
while (ids.length <= index) {
ids.push(`${formId}-array-row-${nextArrayRowId.current++}`);
}
arrayRowIds.current.set(path, ids);
return ids[index] ?? `${formId}-array-row-missing`;
};
const handleSubmit = (event: FormEvent<HTMLFormElement>): void => {
event.preventDefault();
const result = serializeSchemaValues(field, values, sources);
@@ -171,8 +185,10 @@ export const SchemaForm = ({
<form className="schema-form" noValidate onSubmit={handleSubmit}>
{renderBeforeFields}
<SchemaFieldControl
arrayItemKey={arrayItemKey}
diagnostics={allDiagnostics}
field={field}
idPrefix={`schema-form-${formId}`}
onArrayItemRemove={handleArrayItemRemove}
onSourceChange={handleSourceChange}
onValueChange={handleValueChange}
@@ -189,3 +205,8 @@ export const SchemaForm = ({
</form>
);
};
/** Remounts form-local state only when the represented schema changes. */
export const SchemaForm = (props: SchemaFormProps) => (
<SchemaFormState key={rawSchemaText(props.schema)} {...props} />
);