mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-28 00:48:07 +00:00
* feat(clarification): structured form fields for human-input cards Add a request-side v2 `form` mode to the ask_clarification protocol so business flows (e.g. expense reimbursement) can collect several values in one card instead of sequential free-text questions: - `ask_clarification` gains a restricted `fields` parameter (text / textarea / number / select / multi_select / checkbox / date) - ClarificationMiddleware validates and normalizes fields explicitly (whitelisted types, unknown -> text, select-likes without options -> text, duplicate/invalid entries dropped, all-invalid falls back to the legacy modes) since the middleware short-circuits before tool execution; the plain-text fallback lists fields for IM channels - Form payloads carry `version: 2` so older frontends degrade to the text fallback; replies stay on the v1 response protocol — the card submits a readable summary as `response_kind: "text"`, so journal persistence and answered-card recovery are unchanged - Frontend renders typed field controls with required-field validation and compact multi-select chips Part of #4400 (scope narrowed per maintainer feedback: request-side only, no new response kinds, no top-level multi_choice). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): harden form protocol per review feedback Address the five review points on #4406: - Reject field names colliding with JS Object.prototype members on both sides; frontend reads form values via own-property access only, so `constructor`/`toString`-style names can no longer leak inherited members into required validation or the submitted summary - Close open requests answered through the legacy text fallback: a visible plain human reply (no response metadata) now marks every previously-opened request as answered, so upgrading to a v2-aware frontend cannot leave the composer locked on an already-answered card - Give checkbox fields deterministic boolean semantics: values are seeded to an explicit false ("no" in the summary) and `required` means must-agree/consent; documented in the tool schema - Make middleware field validation atomic: structurally broken entries (bad/duplicate/reserved names, over-cap field/option counts or text lengths) degrade the whole form instead of silently dropping fields; options are trimmed/deduped with blanks removed so the backend never emits payloads the frontend parser rejects - Associate form labels/controls (htmlFor/id), aria-required, aria-invalid, and error descriptions for accessibility Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(clarification): type the fields item schema via TypedDict Replace `fields: list[dict[str, Any]]` with `list[ClarificationFormField]` (a TypedDict with `name` required and the type whitelist as a Literal) so the provider-facing tool schema documents the item shape instead of an opaque object relying on the docstring. Runtime validation is unchanged and stays in ClarificationMiddleware, which intercepts the call before tool execution. Addresses the non-blocking review suggestion on #4406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(frontend): drop unsupported aria-invalid from multi-select group jsx-a11y: role=group does not support aria-invalid; the error linkage stays via aria-describedby. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): coerce numeric required flags and normalize fields once - `_normalize_bool` now coerces 1/0 (some providers serialize booleans as integers), so `required: 1` no longer silently flips to optional - `_handle_clarification` normalizes `fields` once and passes the result to both the text fallback and the payload builder Addresses the non-blocking review nits on #4406. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(clarification): harden form protocol per contract review round 2 Backend: - Guard unhashable JSON in the intercept path: `type: []`/`{}` degrades the field to text and `clarification_type: []` coerces to str instead of raising TypeError (which, with return_direct, ended the turn with an error and no card or fallback) - Add a total budget over the serialized normalized fields (16KB UTF-8 bytes): per-item caps alone admitted forms whose IM text fallback exceeded channel delivery limits (Slack 40k chars, Feishu ~30KB card), silently truncating trailing fields; a boundary test proves any accepted form's fallback stays deliverable Frontend: - Submission value now appends a JSON block keyed by stable field names (readable summary alone is delimiter-ambiguous), with a collision regression test - Parser boundary tightened to match backend constraints: empty option values (Radix SelectItem crash), duplicate option ids/values, duplicate field names, and the form<->version-2 binding are rejected - Keep the error node mounted while any field is still invalid so aria-describedby never points at a removed element (happy-dom interaction test) - Required semantics are now accessible: native checkbox control (no HTML required attribute — it would intercept the custom submit path), visually-hidden localized "required" markers next to the aria-hidden asterisks - Legacy-fallback closure narrowed to the latest unanswered request: nothing guarantees a single outstanding clarification across runs, and closing all would silently swallow older decisions; an older request left open becomes the active card again Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(frontend): keep clarification selects controlled --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
572 lines
16 KiB
TypeScript
572 lines
16 KiB
TypeScript
import type { Message } from "@langchain/langgraph-sdk";
|
||
import { expect, test } from "@rstest/core";
|
||
|
||
import {
|
||
buildHumanInputFormSubmissionValue,
|
||
buildHumanInputFormSummary,
|
||
buildHumanInputResponseText,
|
||
buildInitialHumanInputFormValues,
|
||
createHumanInputOptionResponse,
|
||
createHumanInputTextResponse,
|
||
deriveHumanInputThreadState,
|
||
extractHumanInputRequest,
|
||
extractHumanInputResponse,
|
||
hasOpenHumanInputRequest,
|
||
readHumanInputFormValue,
|
||
shouldClearPendingHumanInputOnThreadError,
|
||
} from "@/core/messages/human-input";
|
||
|
||
const requestPayload = {
|
||
version: 1,
|
||
kind: "human_input_request",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
tool_call_id: "call-abc",
|
||
clarification_type: "approach_choice",
|
||
question: "Which environment should I deploy to?",
|
||
context: "Need the target environment.",
|
||
input_mode: "choice_with_other",
|
||
options: [
|
||
{ id: "option-1", label: "development", value: "development" },
|
||
{ id: "option-2", label: "staging", value: "staging" },
|
||
],
|
||
};
|
||
|
||
test("extractHumanInputRequest reads a valid tool artifact payload", () => {
|
||
const message = {
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: {
|
||
human_input: requestPayload,
|
||
},
|
||
} as unknown as Message;
|
||
|
||
expect(extractHumanInputRequest(message)).toEqual(requestPayload);
|
||
});
|
||
|
||
test("extractHumanInputRequest rejects malformed artifacts", () => {
|
||
const message = {
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: {
|
||
human_input: {
|
||
...requestPayload,
|
||
options: [{ id: "option-1", label: "missing value" }],
|
||
},
|
||
},
|
||
} as unknown as Message;
|
||
|
||
expect(extractHumanInputRequest(message)).toBeNull();
|
||
});
|
||
|
||
test("extractHumanInputResponse reads valid human message metadata", () => {
|
||
const response = {
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
response_kind: "option",
|
||
option_id: "option-2",
|
||
value: "staging",
|
||
};
|
||
const message = {
|
||
type: "human",
|
||
content: "For your clarification, my answer is: staging",
|
||
additional_kwargs: {
|
||
hide_from_ui: true,
|
||
human_input_response: response,
|
||
},
|
||
} as unknown as Message;
|
||
|
||
expect(extractHumanInputResponse(message)).toEqual(response);
|
||
});
|
||
|
||
test("derives answered card state from hidden human input responses", () => {
|
||
const response = {
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
response_kind: "option",
|
||
option_id: "option-2",
|
||
value: "staging",
|
||
};
|
||
const state = deriveHumanInputThreadState([
|
||
{
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: {
|
||
human_input: requestPayload,
|
||
},
|
||
} as unknown as Message,
|
||
{
|
||
type: "human",
|
||
content: "For your clarification, my answer is: staging",
|
||
additional_kwargs: {
|
||
hide_from_ui: true,
|
||
human_input_response: response,
|
||
},
|
||
} as unknown as Message,
|
||
]);
|
||
|
||
expect(state.answeredResponses.get("clarification:call-abc")).toEqual(
|
||
response,
|
||
);
|
||
expect(state.latestOpenRequestId).toBeNull();
|
||
});
|
||
|
||
test("detects whether a thread has an open human input request", () => {
|
||
const requestMessage = {
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: {
|
||
human_input: requestPayload,
|
||
},
|
||
} as unknown as Message;
|
||
const responseMessage = {
|
||
type: "human",
|
||
content: "For your clarification, my answer is: staging",
|
||
additional_kwargs: {
|
||
hide_from_ui: true,
|
||
human_input_response: {
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
response_kind: "option",
|
||
option_id: "option-2",
|
||
value: "staging",
|
||
},
|
||
},
|
||
} as unknown as Message;
|
||
|
||
expect(hasOpenHumanInputRequest([requestMessage])).toBe(true);
|
||
expect(hasOpenHumanInputRequest([requestMessage, responseMessage])).toBe(
|
||
false,
|
||
);
|
||
});
|
||
|
||
test("detects new thread errors that should unlock pending human input cards", () => {
|
||
const previousError = new Error("old failure");
|
||
const currentError = new Error("stream failed");
|
||
|
||
expect(
|
||
shouldClearPendingHumanInputOnThreadError({
|
||
currentError,
|
||
pendingRequestCount: 1,
|
||
previousError: undefined,
|
||
}),
|
||
).toBe(true);
|
||
expect(
|
||
shouldClearPendingHumanInputOnThreadError({
|
||
currentError,
|
||
pendingRequestCount: 0,
|
||
previousError: undefined,
|
||
}),
|
||
).toBe(false);
|
||
expect(
|
||
shouldClearPendingHumanInputOnThreadError({
|
||
currentError: previousError,
|
||
pendingRequestCount: 1,
|
||
previousError,
|
||
}),
|
||
).toBe(false);
|
||
expect(
|
||
shouldClearPendingHumanInputOnThreadError({
|
||
currentError: undefined,
|
||
pendingRequestCount: 1,
|
||
previousError: currentError,
|
||
}),
|
||
).toBe(false);
|
||
});
|
||
|
||
test("creates option and text responses for a request", () => {
|
||
const request = extractHumanInputRequest({
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: {
|
||
human_input: requestPayload,
|
||
},
|
||
} as unknown as Message);
|
||
|
||
expect(request).not.toBeNull();
|
||
const optionResponse = createHumanInputOptionResponse(
|
||
request!,
|
||
request!.options![1]!,
|
||
);
|
||
const textResponse = createHumanInputTextResponse(
|
||
request!,
|
||
"Use blue-green deployment",
|
||
);
|
||
|
||
expect(optionResponse).toEqual({
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
response_kind: "option",
|
||
option_id: "option-2",
|
||
value: "staging",
|
||
});
|
||
expect(textResponse).toEqual({
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-abc",
|
||
response_kind: "text",
|
||
value: "Use blue-green deployment",
|
||
});
|
||
expect(buildHumanInputResponseText(request!, optionResponse)).toBe(
|
||
'For your clarification "Which environment should I deploy to?", my answer is: staging',
|
||
);
|
||
});
|
||
|
||
const formPayload = {
|
||
version: 2,
|
||
kind: "human_input_request",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-form",
|
||
question: "Please provide the expense details.",
|
||
input_mode: "form",
|
||
fields: [
|
||
{ name: "amount", label: "Amount", type: "number", required: true },
|
||
{
|
||
name: "category",
|
||
label: "Category",
|
||
type: "select",
|
||
required: true,
|
||
options: [
|
||
{ id: "category-option-1", label: "travel", value: "travel" },
|
||
{ id: "category-option-2", label: "meals", value: "meals" },
|
||
],
|
||
},
|
||
{
|
||
name: "receipts",
|
||
label: "Receipts",
|
||
type: "multi_select",
|
||
required: false,
|
||
options: [
|
||
{ id: "receipts-option-1", label: "A-1", value: "A-1" },
|
||
{ id: "receipts-option-2", label: "A-2", value: "A-2" },
|
||
],
|
||
},
|
||
{ name: "note", label: "note", type: "textarea", required: false },
|
||
],
|
||
};
|
||
|
||
function toolMessage(payload: unknown): Message {
|
||
return {
|
||
type: "tool",
|
||
name: "ask_clarification",
|
||
content: "fallback",
|
||
artifact: { human_input: payload },
|
||
} as unknown as Message;
|
||
}
|
||
|
||
test("parses a v2 form request", () => {
|
||
expect(extractHumanInputRequest(toolMessage(formPayload))).toEqual(
|
||
formPayload,
|
||
);
|
||
});
|
||
|
||
test("rejects a form request without fields", () => {
|
||
expect(
|
||
extractHumanInputRequest(toolMessage({ ...formPayload, fields: [] })),
|
||
).toBeNull();
|
||
});
|
||
|
||
test("rejects a form request with malformed fields", () => {
|
||
expect(
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [{ label: "missing name", type: "text" }],
|
||
}),
|
||
),
|
||
).toBeNull();
|
||
expect(
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [{ name: "amount", label: "Amount", type: "slider" }],
|
||
}),
|
||
),
|
||
).toBeNull();
|
||
});
|
||
|
||
test("rejects unknown protocol versions", () => {
|
||
expect(
|
||
extractHumanInputRequest(toolMessage({ ...formPayload, version: 3 })),
|
||
).toBeNull();
|
||
});
|
||
|
||
test("builds a readable form summary submitted as a v1 text response", () => {
|
||
const request = extractHumanInputRequest(toolMessage(formPayload))!;
|
||
const values = {
|
||
amount: "300",
|
||
category: "travel",
|
||
receipts: ["A-1", "A-2"],
|
||
note: "",
|
||
};
|
||
|
||
const summary = buildHumanInputFormSummary(request, values);
|
||
expect(summary).toBe("Amount: 300; Category: travel; Receipts: A-1, A-2");
|
||
|
||
// Request-side-only protocol scope: form answers reuse the existing v1
|
||
// text response — no structured response kind is introduced.
|
||
expect(createHumanInputTextResponse(request, summary)).toEqual({
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-form",
|
||
response_kind: "text",
|
||
value: "Amount: 300; Category: travel; Receipts: A-1, A-2",
|
||
});
|
||
});
|
||
|
||
test("form submission value is collision-free via the JSON block", () => {
|
||
const request = extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{ name: "a", label: "A", type: "text", required: false },
|
||
{ name: "b", label: "B", type: "text", required: false },
|
||
],
|
||
}),
|
||
)!;
|
||
|
||
const first = buildHumanInputFormSubmissionValue(request, {
|
||
a: "x; B: y",
|
||
b: "z",
|
||
});
|
||
const second = buildHumanInputFormSubmissionValue(request, {
|
||
a: "x",
|
||
b: "y; B: z",
|
||
});
|
||
|
||
expect(first).not.toBe(second);
|
||
expect(first).toContain('[values: {"a":"x; B: y","b":"z"}]');
|
||
expect(second).toContain('[values: {"a":"x","b":"y; B: z"}]');
|
||
// Readable prefix is retained for display.
|
||
expect(first.startsWith("A: x; B: y; B: z")).toBe(true);
|
||
});
|
||
|
||
test("rejects select options with empty values or duplicates", () => {
|
||
const withOptions = (
|
||
options: Array<{ id: string; label: string; value: string }>,
|
||
) =>
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{
|
||
name: "category",
|
||
label: "Category",
|
||
type: "select",
|
||
required: true,
|
||
options,
|
||
},
|
||
],
|
||
}),
|
||
);
|
||
|
||
// Empty option value would crash Radix <SelectItem value="">.
|
||
expect(withOptions([{ id: "o1", label: "travel", value: "" }])).toBeNull();
|
||
// Duplicate option ids / values.
|
||
expect(
|
||
withOptions([
|
||
{ id: "o1", label: "travel", value: "travel" },
|
||
{ id: "o1", label: "meals", value: "meals" },
|
||
]),
|
||
).toBeNull();
|
||
expect(
|
||
withOptions([
|
||
{ id: "o1", label: "travel", value: "travel" },
|
||
{ id: "o2", label: "travel-again", value: "travel" },
|
||
]),
|
||
).toBeNull();
|
||
});
|
||
|
||
test("rejects duplicate field names and enforces the form/version binding", () => {
|
||
expect(
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{ name: "amount", label: "Amount", type: "number", required: true },
|
||
{ name: "amount", label: "Again", type: "text", required: false },
|
||
],
|
||
}),
|
||
),
|
||
).toBeNull();
|
||
// form mode is a v2 construct; a v1 payload carrying it is malformed.
|
||
expect(
|
||
extractHumanInputRequest(toolMessage({ ...formPayload, version: 1 })),
|
||
).toBeNull();
|
||
// and v2 without form has no defined meaning yet.
|
||
expect(
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...requestPayload,
|
||
version: 2,
|
||
}),
|
||
),
|
||
).toBeNull();
|
||
});
|
||
|
||
test("rejects form fields with reserved prototype names", () => {
|
||
for (const reserved of ["__proto__", "constructor", "toString"]) {
|
||
expect(
|
||
extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{ name: reserved, label: "Amount", type: "number", required: true },
|
||
],
|
||
}),
|
||
),
|
||
).toBeNull();
|
||
}
|
||
});
|
||
|
||
test("readHumanInputFormValue ignores inherited prototype properties", () => {
|
||
const values: Record<string, string> = { amount: "300" };
|
||
|
||
expect(readHumanInputFormValue(values, "amount")).toBe("300");
|
||
expect(readHumanInputFormValue(values, "toString")).toBeUndefined();
|
||
expect(readHumanInputFormValue(values, "constructor")).toBeUndefined();
|
||
expect(readHumanInputFormValue(values, "__proto__")).toBeUndefined();
|
||
});
|
||
|
||
test("buildInitialHumanInputFormValues seeds checkbox fields to false", () => {
|
||
const request = extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{ name: "amount", label: "Amount", type: "number", required: true },
|
||
{ name: "urgent", label: "Urgent", type: "checkbox", required: false },
|
||
],
|
||
}),
|
||
)!;
|
||
|
||
expect(buildInitialHumanInputFormValues(request.fields ?? [])).toEqual({
|
||
urgent: false,
|
||
});
|
||
});
|
||
|
||
test("summary renders an untouched checkbox as an explicit no", () => {
|
||
const request = extractHumanInputRequest(
|
||
toolMessage({
|
||
...formPayload,
|
||
fields: [
|
||
{ name: "amount", label: "Amount", type: "number", required: true },
|
||
{ name: "urgent", label: "Urgent", type: "checkbox", required: false },
|
||
],
|
||
}),
|
||
)!;
|
||
const values = {
|
||
amount: "300",
|
||
...buildInitialHumanInputFormValues(request.fields ?? []),
|
||
};
|
||
|
||
expect(buildHumanInputFormSummary(request, values)).toBe(
|
||
"Amount: 300; Urgent: no",
|
||
);
|
||
});
|
||
|
||
test("a plain reply closes only the latest unanswered request", () => {
|
||
const olderPayload = {
|
||
...formPayload,
|
||
request_id: "clarification:call-older",
|
||
};
|
||
const state = deriveHumanInputThreadState([
|
||
toolMessage(olderPayload),
|
||
toolMessage(formPayload),
|
||
{
|
||
type: "human",
|
||
content: "answer to the second question",
|
||
} as unknown as Message,
|
||
]);
|
||
|
||
// The reply answers the request the user was looking at (the latest); the
|
||
// older decision must not be silently swallowed with the same text.
|
||
expect(state.answeredResponses.get("clarification:call-form")?.value).toBe(
|
||
"answer to the second question",
|
||
);
|
||
expect(state.answeredResponses.has("clarification:call-older")).toBe(false);
|
||
expect(state.latestOpenRequestId).toBe("clarification:call-older");
|
||
});
|
||
|
||
test("a visible plain human reply closes an open request (legacy fallback)", () => {
|
||
// An old (v1-only) frontend renders a v2 request as plain text and the user
|
||
// answers through the normal composer — the reply has no response metadata.
|
||
// After upgrading, the request must not stay open and lock the composer.
|
||
const state = deriveHumanInputThreadState([
|
||
toolMessage(formPayload),
|
||
{
|
||
type: "human",
|
||
content: "金额 300,类别差旅",
|
||
} as unknown as Message,
|
||
]);
|
||
|
||
expect(state.latestOpenRequestId).toBeNull();
|
||
const answered = state.answeredResponses.get("clarification:call-form");
|
||
expect(answered?.response_kind).toBe("text");
|
||
expect(answered?.value).toBe("金额 300,类别差旅");
|
||
expect(hasOpenHumanInputRequest([toolMessage(formPayload)])).toBe(true);
|
||
});
|
||
|
||
test("a visible human message before the request does not close it", () => {
|
||
const state = deriveHumanInputThreadState([
|
||
{
|
||
type: "human",
|
||
content: "帮我提交报销",
|
||
} as unknown as Message,
|
||
toolMessage(formPayload),
|
||
]);
|
||
|
||
expect(state.latestOpenRequestId).toBe("clarification:call-form");
|
||
});
|
||
|
||
test("hidden non-response messages do not close an open request", () => {
|
||
const state = deriveHumanInputThreadState([
|
||
toolMessage(formPayload),
|
||
{
|
||
type: "human",
|
||
content: "internal context",
|
||
additional_kwargs: { hide_from_ui: true },
|
||
} as unknown as Message,
|
||
]);
|
||
|
||
expect(state.latestOpenRequestId).toBe("clarification:call-form");
|
||
});
|
||
|
||
test("derives answered state for a form request answered by text", () => {
|
||
const response = {
|
||
version: 1,
|
||
kind: "human_input_response",
|
||
source: "ask_clarification",
|
||
request_id: "clarification:call-form",
|
||
response_kind: "text",
|
||
value: "Amount: 300",
|
||
};
|
||
const state = deriveHumanInputThreadState([
|
||
toolMessage(formPayload),
|
||
{
|
||
type: "human",
|
||
content: "answer",
|
||
additional_kwargs: { hide_from_ui: true, human_input_response: response },
|
||
} as unknown as Message,
|
||
]);
|
||
|
||
expect(state.answeredResponses.get("clarification:call-form")).toEqual(
|
||
response,
|
||
);
|
||
expect(state.latestOpenRequestId).toBeNull();
|
||
});
|