fix: harden workflow console review paths

This commit is contained in:
lda
2026-08-12 14:23:10 +07:00 Verified
parent 447b8d9e89
commit 20e6ecc117
20 changed files with 416 additions and 56 deletions
+2 -3
View File
@@ -1711,15 +1711,14 @@ tbody tr:hover {
.capability-palette,
.context-inspector,
.authoring-graph {
min-height: 42rem;
max-height: calc(100vh - 10rem);
min-height: min(42rem, calc(100dvh - 10rem));
max-height: calc(100dvh - 10rem);
overflow: auto;
padding: 0.85rem;
border: 1px solid var(--color-border);
border-radius: 4px;
background: var(--color-surface);
}
.capability-palette__header,
.authoring-graph__heading,
.context-inspector__heading {
@@ -1,4 +1,4 @@
import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { useRef } from "react";
import { MemoryRouter, Outlet, Route, Routes, useNavigate } from "react-router-dom";
@@ -240,11 +240,12 @@ describe("ConsoleWorkspace", () => {
});
oldWrite.resolve(successfulWrite());
await waitFor(() => {
expect(screen.getByTestId("evidence-ids")).toHaveTextContent(
"workflow.health-0|workflow.health-1",
);
await act(async () => {
await oldWrite.promise;
});
expect(screen.getByTestId("evidence-ids")).toHaveTextContent(
"workflow.health-0|workflow.health-1",
);
});
it("ignores a stale health response after a newer target connects", async () => {
@@ -168,7 +168,16 @@ export const ContextInspector = ({
<h2 id="capability-selection-heading">{selection.qualifiedName}</h2>
<p>{capability?.description ?? "Configure this capability before adding it."}</p>
<dl className="authoring-inspector__facts">
<Fact label="Kind" value={capability?.kind === "wrapper_artifact" ? "Wrapper artifact" : "Node spec"} />
<Fact
label="Kind"
value={
capability === undefined
? "unknown"
: capability.kind === "wrapper_artifact"
? "Wrapper artifact"
: "Node spec"
}
/>
<Fact label="Outcomes" value={capability?.outcomes.join(", ") || "none"} />
</dl>
</section>
@@ -129,10 +129,10 @@ export const CreateDraftDialog = ({
return;
}
setPhase("saving");
setMessage(null);
const lifecycleToken = lifecycleTokenRef.current;
if (lifecycleToken === null) return;
setPhase("saving");
setMessage(null);
const requestGeneration = requestGenerationRef.current + 1;
requestGenerationRef.current = requestGeneration;
try {
@@ -1,6 +1,6 @@
import { cleanup, render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { InputBinding } from "../domain/draft-workspace-models.js";
import { StepInputBindingsForm } from "./StepInputBindingsForm.js";
import { displayGraphInputPath, displayLocalInputPath } from "./input-binding-paths.js";
@@ -97,6 +97,26 @@ describe("StepInputBindingsForm", () => {
await user.click(screen.getByRole("button", { name: "Clear inputs" }));
expect(submissions).toEqual([[]]);
expect(screen.queryByRole("group", { name: "Input row 1" })).toBeNull();
});
it("keeps input rows when clearing fails", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn().mockRejectedValue(new Error("clear failed"));
render(
<StepInputBindingsForm
inputSchema={schema}
initialRows={[
{ kind: "canonical", index: 0, value: { path: "input.title", target: "title" } },
]}
onSubmit={onSubmit}
/>,
);
await user.click(screen.getByRole("button", { name: "Clear inputs" }));
expect(onSubmit).toHaveBeenCalledWith([]);
expect(screen.getByRole("group", { name: "Input row 1" })).toBeInTheDocument();
});
it("blocks clear until every unsupported row is explicitly removed", async () => {
@@ -329,7 +329,8 @@ export const StepInputBindingsForm = ({
const clear = (): void => {
if (unsupportedRows.length > 0) {
const message = "Remove or repair this unsupported input row before clearing inputs.";
const message =
"Remove or repair this unsupported input row before clearing inputs.";
setFormIssue(message);
markDirty();
return;
@@ -337,9 +338,10 @@ export const StepInputBindingsForm = ({
setLocalIssues({});
setFormIssue(null);
markDirty();
void Promise.resolve(onSubmit([])).catch(() => undefined);
void Promise.resolve(onSubmit([]))
.then(() => setRows([]))
.catch(() => undefined);
};
return (
<form className="schema-form authoring-form" noValidate onSubmit={submit}>
{formIssue !== null && <p id={formErrorId} role="alert">{formIssue}</p>}
@@ -32,22 +32,35 @@ const hasExactKeys = (value: JsonRecord, keys: ReadonlyArray<string>): boolean =
};
/** Guard the recursive JSON subset used by literal input bindings. */
export const isJsonValue = (value: unknown): value is JsonValue => {
if (value === null || typeof value === "boolean" || typeof value === "string") return true;
const MAX_JSON_DEPTH = 64;
const isJsonValueAtDepth = (value: unknown, depth: number): boolean => {
if (depth > MAX_JSON_DEPTH) return false;
if (value === null || typeof value === "boolean" || typeof value === "string")
return true;
if (typeof value === "number") return Number.isFinite(value);
if (Array.isArray(value)) {
if (Object.getOwnPropertySymbols(value).length > 0) return false;
for (const item of value) {
if (!isJsonValue(item)) return false;
if (!isJsonValueAtDepth(item, depth + 1)) return false;
}
return Object.keys(value).every((key) => /^(0|[1-9]\d*)$/.test(key));
}
if (!isRecord(value)) return false;
if (Object.getPrototypeOf(value) !== Object.prototype && Object.getPrototypeOf(value) !== null) return false;
if (
Object.getPrototypeOf(value) !== Object.prototype &&
Object.getPrototypeOf(value) !== null
)
return false;
if (Object.getOwnPropertySymbols(value).length > 0) return false;
return Object.values(value).every(isJsonValue);
return Object.values(value).every((item) =>
isJsonValueAtDepth(item, depth + 1),
);
};
/** Guard the recursive JSON subset used by literal input bindings. */
export const isJsonValue = (value: unknown): value is JsonValue =>
isJsonValueAtDepth(value, 0);
const stringParts = (value: unknown): string[] | null => {
if (!Array.isArray(value)) return null;
const parts: string[] = [];
@@ -306,6 +306,16 @@ describe("useDraftAuthoring", () => {
expect(result.current.preservedCapabilityForm).toEqual({ kind: "update", input });
});
it("publishes remembered capability form changes to consumers", () => {
const initial = workspace();
const { result } = renderHook(() => useDraftAuthoring({ draft: initial }));
const input = { ...capabilityInput, description: "Unsaved operator edit" };
act(() => result.current.rememberCapabilityForm("add", input));
expect(result.current.preservedCapabilityForm).toEqual({ kind: "add", input });
});
it("does not send different writes or validation concurrently for one revision", async () => {
const initial = workspace({ revision: 6 });
let resolveAdd: ((value: DraftWorkspace) => void) | undefined;
@@ -328,6 +338,68 @@ describe("useDraftAuthoring", () => {
await act(async () => first);
});
it("rejects reload while a mutation is in progress", async () => {
const initial = workspace({ revision: 6 });
let resolveAdd: ((value: DraftWorkspace) => void) | undefined;
authoringClient.addCapabilityStep.mockReturnValueOnce(
new Promise<DraftWorkspace>((resolve) => { resolveAdd = resolve; }),
);
const { result } = renderHook(() => useDraftAuthoring({ draft: initial }));
let mutation: Promise<void> | undefined;
act(() => { mutation = result.current.addCapability(capabilityInput); });
await expect(result.current.reload()).rejects.toThrow(
"Another draft authoring request is in progress.",
);
expect(workspaceClient.load).not.toHaveBeenCalled();
resolveAdd?.(workspace({ revision: 7 }));
await act(async () => mutation);
});
it("rejects mutations while reload is in progress", async () => {
const initial = workspace({ revision: 6 });
let resolveReload: ((value: DraftWorkspace) => void) | undefined;
workspaceClient.load.mockReturnValueOnce(
new Promise<DraftWorkspace>((resolve) => { resolveReload = resolve; }),
);
const { result } = renderHook(() => useDraftAuthoring({ draft: initial }));
let reloadRequest: Promise<void> | undefined;
act(() => { reloadRequest = result.current.reload(); });
await expect(result.current.addCapability(capabilityInput)).rejects.toThrow(
"Another draft authoring request is in progress.",
);
expect(authoringClient.addCapabilityStep).not.toHaveBeenCalled();
resolveReload?.(workspace({ revision: 7 }));
await act(async () => reloadRequest);
});
it("coalesces duplicate reload requests for the same revision", async () => {
const initial = workspace({ revision: 6 });
let resolveReload: ((value: DraftWorkspace) => void) | undefined;
workspaceClient.load.mockReturnValueOnce(
new Promise<DraftWorkspace>((resolve) => { resolveReload = resolve; }),
);
const { result } = renderHook(() => useDraftAuthoring({ draft: initial }));
let first: Promise<void> | undefined;
let second: Promise<void> | undefined;
act(() => {
first = result.current.reload();
second = result.current.reload();
});
expect(first).toBe(second);
expect(workspaceClient.load).toHaveBeenCalledTimes(1);
resolveReload?.(workspace({ revision: 7 }));
await act(async () => first);
});
it("coalesces duplicate validation requests for the loaded revision", async () => {
const initial = workspace({ revision: 8 });
let resolveValidation: ((value: DraftWorkspace) => void) | undefined;
@@ -382,6 +454,26 @@ describe("useDraftAuthoring", () => {
);
});
it("reapplies an add with its original connector insertion context", async () => {
const initial = workspace();
const conflict = workspace({ revision: 4, status: "conflict" });
authoringClient.addCapabilityStep
.mockResolvedValueOnce(conflict)
.mockResolvedValueOnce(workspace({ revision: 5 }));
const { result } = renderHook(() => useDraftAuthoring({
draft: initial,
initialSelection: { kind: "edge", stepId: "read", outcome: "ok" },
}));
await act(async () => result.current.addCapability(capabilityInput));
act(() => result.current.select({ kind: "edge", stepId: "fallback", outcome: "error" }));
await act(async () => result.current.reapply());
expect(authoringClient.addCapabilityStep).toHaveBeenLastCalledWith(
expect.objectContaining({ routeFromStep: "read", routeFromOutcome: "ok" }),
);
});
it("reapplies a conflicted update to its original node after selection changes", async () => {
const initial = workspace({
draft: {
@@ -465,6 +557,38 @@ describe("useDraftAuthoring", () => {
expect(result.current.dirty).toBe(true);
});
it("ignores a stale targeted response after provenance and selection both change", async () => {
const initial = workspace({ revision: 7 });
let resolveInputs: ((value: DraftWorkspace) => void) | undefined;
setStepInputBindings.mockReturnValueOnce(
new Promise<DraftWorkspace>((resolve) => { resolveInputs = resolve; }),
);
const { result, rerender } = renderHook(() => useDraftAuthoring({
draft: initial,
initialSelection: { kind: "node", nodeId: "render" },
}));
const request = result.current.setStepInputs([{ path: "input.title", target: "title" }]);
act(() => result.current.select({ kind: "node", nodeId: "publish" }));
contextValue = { ...contextValue, connectedTarget: "server-b" };
rerender();
const stateBeforeResponse = {
dirty: result.current.dirty,
phase: result.current.phase,
message: result.current.message,
};
resolveInputs?.(workspace({ revision: 8 }));
await act(async () => request);
expect(result.current.draft).toBe(initial);
expect(result.current.selection).toEqual({ kind: "node", nodeId: "publish" });
expect({
dirty: result.current.dirty,
phase: result.current.phase,
message: result.current.message,
}).toEqual(stateBeforeResponse);
});
it("submits selected-step inputs against the selected node and current revision", async () => {
const initial = workspace({ revision: 7 });
const canonical = workspace({ revision: 8 });
@@ -592,6 +716,39 @@ describe("useDraftAuthoring", () => {
expect(result.current.draft).toBe(initial);
expect(result.current.selection).toEqual({ kind: "node", nodeId: "publish" });
expect(result.current.phase).toBe("idle");
expect(result.current.message).toBeNull();
});
it("ignores a capability update response after selecting another node", async () => {
const initial = workspace({
revision: 7,
draft: {
steps: { read: { use: "demo.read" }, publish: { use: "demo.publish" } },
routes: {},
},
});
let resolveUpdate: ((value: DraftWorkspace) => void) | undefined;
updateCapabilityStep.mockReturnValueOnce(
new Promise<DraftWorkspace>((resolve) => { resolveUpdate = resolve; }),
);
const { result } = renderHook(() => useDraftAuthoring({
draft: initial,
initialSelection: { kind: "node", nodeId: "read" },
}));
const request = result.current.updateCapability({
...capabilityInput,
stepId: "read",
capabilityName: "demo.read",
});
act(() => result.current.select({ kind: "node", nodeId: "publish" }));
resolveUpdate?.(workspace({ revision: 8 }));
await act(async () => request);
expect(result.current.draft).toBe(initial);
expect(result.current.selection).toEqual({ kind: "node", nodeId: "publish" });
expect(result.current.phase).toBe("idle");
});
it("reapplies the exact input submission to its original step after reload", async () => {
@@ -81,7 +81,11 @@ type PendingMutation = {
};
type LastSubmission =
| { readonly kind: "add"; readonly input: CapabilityNodeFormValue }
| {
readonly kind: "add";
readonly input: CapabilityNodeFormValue;
readonly insertion: InsertionContext | null;
}
| {
readonly kind: "update";
readonly targetStepId: string;
@@ -242,6 +246,8 @@ export const useDraftAuthoring = ({
// tabs and unsaved rows; this ref stores only the exact mutation payload
// needed for an explicit conflict reapply.
const lastSubmissionRef = useRef<LastSubmission>(null);
const [preservedCapabilityForm, setPreservedCapabilityForm] =
useState<PreservedCapabilityForm>(null);
const adoptsDraftInput =
state.draftInput !== initialDraft &&
@@ -362,6 +368,11 @@ export const useDraftAuthoring = ({
if (options.submission !== undefined) {
lastSubmissionRef.current = options.submission;
setPreservedCapabilityForm(
options.submission.kind === "add" || options.submission.kind === "update"
? { kind: options.submission.kind, input: options.submission.input }
: null,
);
}
const requestProvenance = currentProvenanceRef.current;
setState((current) => ({
@@ -372,14 +383,24 @@ export const useDraftAuthoring = ({
}));
const promise = operation(authoringClient, requestDraft)
.then((response) => {
if (!sameProvenance(requestProvenance, currentProvenanceRef.current)) return;
const targetIsCurrent =
options.targetStepId === undefined ||
options.allowTargetSelectionChange === true ||
(currentSelectionRef.current.kind === "node" &&
currentSelectionRef.current.nodeId === options.targetStepId);
const committed =
targetIsCurrent &&
commitResponse(response, requestProvenance, options.nextSelection);
if (!targetIsCurrent) {
// Do not reset the newly selected inspector with a response for the
// previous node. The canonical server state remains reloadable.
setState((current) => ({
...current,
dirty: true,
phase: "idle",
message: null,
}));
return;
}
const committed = commitResponse(response, requestProvenance, options.nextSelection);
if (!committed && sameProvenance(requestProvenance, currentProvenanceRef.current)) {
throw new Error("The authoring response did not match the requested workspace.");
}
@@ -402,9 +423,11 @@ export const useDraftAuthoring = ({
[authoringClient, commitResponse],
);
const addCapability = useCallback(
(input: CapabilityNodeFormValue): Promise<void> => {
const insertion = currentInsertionContextRef.current;
const submitCapabilityAdd = useCallback(
(
input: CapabilityNodeFormValue,
insertion: InsertionContext | null,
): Promise<void> => {
return runMutation(
"add",
{ input, insertion },
@@ -430,15 +453,25 @@ export const useDraftAuthoring = ({
}),
{
nextSelection: { kind: "node", nodeId: input.stepId },
submission: { kind: "add", input },
submission: { kind: "add", input, insertion },
},
);
},
[runMutation],
);
const addCapability = useCallback(
(input: CapabilityNodeFormValue): Promise<void> =>
submitCapabilityAdd(input, currentInsertionContextRef.current),
[submitCapabilityAdd],
);
const submitCapabilityUpdate = useCallback(
(targetStepId: string, input: CapabilityNodeFormValue): Promise<void> => {
(
targetStepId: string,
input: CapabilityNodeFormValue,
allowTargetSelectionChange = false,
): Promise<void> => {
// Persist the immutable mutation target with the form. A conflict may
// outlive the current graph selection before the operator reapplies it.
return runMutation(
@@ -457,6 +490,8 @@ export const useDraftAuthoring = ({
},
}),
{
targetStepId,
allowTargetSelectionChange,
submission: { kind: "update", targetStepId, input },
},
);
@@ -673,8 +708,17 @@ export const useDraftAuthoring = ({
const reload = useCallback((): Promise<void> => {
if (!workspaceClient) return Promise.resolve();
const requestProvenance = currentProvenanceRef.current;
return workspaceClient
.load(currentDraftRef.current.workspaceId)
const workspaceId = currentDraftRef.current.workspaceId;
const key = mutationKey("reload", workspaceId, currentDraftRef.current.revision);
const pending = pendingRef.current;
if (pending?.key === key) return pending.promise;
if (pending !== null) {
const error = new Error("Another draft authoring request is in progress.");
setState((current) => ({ ...current, phase: "error", message: error.message }));
return Promise.reject(error);
}
const promise = workspaceClient
.load(workspaceId)
.then((response) => {
if (
!sameProvenance(requestProvenance, currentProvenanceRef.current) ||
@@ -702,21 +746,28 @@ export const useDraftAuthoring = ({
.catch((error: unknown) => {
if (!sameProvenance(requestProvenance, currentProvenanceRef.current)) return;
setState((current) => ({ ...current, phase: "error", message: errorMessage(error) }));
})
.finally(() => {
if (pendingRef.current?.promise === promise) pendingRef.current = null;
});
pendingRef.current = { key, promise };
return promise;
}, [initialDraft, workspaceClient]);
const reapply = useCallback((): Promise<void> => {
const last = lastSubmissionRef.current;
if (last === null) return Promise.resolve();
if (last.kind === "add") return addCapability(last.input);
if (last.kind === "update") return submitCapabilityUpdate(last.targetStepId, last.input);
if (last.kind === "add") return submitCapabilityAdd(last.input, last.insertion);
if (last.kind === "update") {
return submitCapabilityUpdate(last.targetStepId, last.input, true);
}
if (last.kind === "route") return setRoute(last.input);
if (last.kind === "setup") return submitSetup(last.targetStepId, last.patch, true);
if (last.kind === "inputs") return submitStepInputs(last.targetStepId, last.bindings, true);
return submitStepOutputs(last.targetStepId, last.bindings, true);
}, [
addCapability,
setRoute,
submitCapabilityAdd,
submitCapabilityUpdate,
submitSetup,
submitStepInputs,
@@ -726,7 +777,13 @@ export const useDraftAuthoring = ({
const rememberCapabilityForm = useCallback(
(kind: "add" | "update", input: CapabilityNodeFormValue): void => {
if (kind === "add") {
lastSubmissionRef.current = { kind, input };
const previous = lastSubmissionRef.current;
const insertion =
previous?.kind === "add"
? previous.insertion
: currentInsertionContextRef.current;
lastSubmissionRef.current = { kind, input, insertion };
setPreservedCapabilityForm({ kind, input });
return;
}
const previous = lastSubmissionRef.current;
@@ -737,12 +794,14 @@ export const useDraftAuthoring = ({
? currentSelectionRef.current.nodeId
: input.stepId;
lastSubmissionRef.current = { kind, targetStepId, input };
setPreservedCapabilityForm({ kind, input });
},
[],
);
const rememberRouteForm = useCallback((input: RouteFormValue): void => {
lastSubmissionRef.current = { kind: "route", input };
setPreservedCapabilityForm(null);
}, []);
return {
@@ -762,13 +821,7 @@ export const useDraftAuthoring = ({
validate,
reload,
reapply,
preservedCapabilityForm:
lastSubmissionRef.current?.kind === "add" || lastSubmissionRef.current?.kind === "update"
? {
kind: lastSubmissionRef.current.kind,
input: lastSubmissionRef.current.input,
}
: null,
preservedCapabilityForm,
rememberCapabilityForm,
rememberRouteForm,
select,
@@ -302,8 +302,24 @@ const fitValue = (value: unknown, maxBytes: number): unknown => {
export const sanitizeEvidenceValue = (value: unknown): unknown =>
fitValue(projectValue(value, 0, new WeakSet()), EVIDENCE_MAX_BYTES);
const sanitizeTarget = (target: string): string => {
try {
const url = new URL(target);
if (url.username !== "" || url.password !== "") {
url.username = "";
url.password = "";
return fitString(url.toString(), MAX_STRING_LENGTH);
}
} catch {
// Fall through: a non-absolute target is bounded but not rewritten.
}
return fitString(target, MAX_STRING_LENGTH);
};
export const sanitizeEvidenceRecord = (record: EvidenceRecord): EvidenceRecord => ({
...record,
target: sanitizeTarget(record.target),
label: fitString(record.label, MAX_STRING_LENGTH),
equivalentCli: requestContainsSensitiveKey(record.request)
? SENSITIVE_REQUEST_CLI_MARKER
: fitCliString(record.equivalentCli),
@@ -214,13 +214,21 @@ const ResultReceipt = ({
<div className="capability-playground__receipt-heading">
<div>
<p className="capability-playground__receipt-label">Result receipt</p>
<h3 id="capability-playground-result-heading">{result.qualifiedName}</h3>
<h3 id="capability-playground-result-heading">
{result.qualifiedName}
</h3>
</div>
<p
className="capability-playground__outcome"
data-outcome={result.outcome === "runtime_error" ? "runtime-error" : "completed"}
data-outcome={
result.outcome === "runtime_error" ? "runtime-error" : "completed"
}
>
<CheckCircle2 aria-hidden="true" size={17} strokeWidth={2} />
{result.outcome === "runtime_error" ? (
<AlertCircle aria-hidden="true" size={17} strokeWidth={2} />
) : (
<CheckCircle2 aria-hidden="true" size={17} strokeWidth={2} />
)}
<span>{outcomeLabel(result.outcome)}</span>
</p>
</div>
@@ -13,9 +13,9 @@ export const DraftDetailRoute = ({
}: DraftDetailRouteProps) => {
const { workspaceId = null } = useParams<{ workspaceId: string }>();
const [searchParams] = useSearchParams();
const capabilityName = searchParams.get("capability");
const capabilityName = searchParams.get("capability")?.trim() ?? "";
const initialSelection: WorkbenchSelection =
capabilityName !== null && capabilityName.trim() !== ""
capabilityName !== ""
? { kind: "capability", qualifiedName: capabilityName }
: { kind: "canvas" };
const drafts = useDraftWorkspace(workspaceId);
@@ -125,10 +125,7 @@ export const SchemaForm = ({
...sources,
[sourceKey(changedField)]: { mode: "literal", value: nextValue },
};
setSources(() => ({
...sources,
[sourceKey(changedField)]: { mode: "literal", value: nextValue },
}));
setSources(nextSources);
}
onValueChange?.(serializeSchemaValues(field, nextValues, nextSources));
};