阿泽 1baa8ad696
feat(clarification): structured form fields for human-input cards (#4400 Phase 1) (#4406)
* 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>
2026-07-27 14:05:31 +08:00

248 lines
7.2 KiB
TypeScript

import { describe, expect, it } from "@rstest/core";
import { createElement, type KeyboardEvent } from "react";
import { renderToStaticMarkup } from "react-dom/server";
import {
findMissingRequiredFields,
HumanInputCard,
shouldSubmitHumanInputTextOnKeyDown,
} from "@/components/workspace/messages/human-input-card";
import { I18nContext } from "@/core/i18n/context";
import type {
HumanInputRequest,
HumanInputResponse,
} from "@/core/messages/human-input";
const request: HumanInputRequest = {
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" },
],
};
describe("HumanInputCard", () => {
it("renders request text, options, and the other-answer input", () => {
const html = renderCard();
expect(html).toContain("Need your help");
expect(html).toContain("Need the target environment.");
expect(html).toContain("Which environment should I deploy to?");
expect(html).toContain("development");
expect(html).toContain("staging");
expect(html).toContain("Other answer");
expect(html).toContain("Type another answer...");
});
it("renders answered state as disabled with the selected value", () => {
const response: HumanInputResponse = {
version: 1,
kind: "human_input_response",
source: "ask_clarification",
request_id: "clarification:call-abc",
response_kind: "option",
option_id: "option-2",
value: "staging",
};
const html = renderCard({ answeredResponse: response });
expect(html).toContain("Answered");
expect(html).toContain("Answered: staging");
expect(html).toContain("disabled");
});
it("renders read-only state when no submit handler is available", () => {
const html = renderCard({ onSubmit: undefined });
expect(html).toContain("Read only");
expect(html).toContain("disabled");
});
it("renders markdown in question field (bold, lists)", () => {
const html = renderCard({
request: {
...request,
question:
"你想写什么样的小说?\n\n1. **题材/类型**:科幻、奇幻\n2. **篇幅**:短篇、中篇",
input_mode: "free_text",
options: undefined,
},
});
expect(html).toContain("题材/类型");
expect(html).toContain("篇幅");
expect(html).not.toContain("**题材/类型**");
expect(html).not.toContain("**篇幅**");
});
it("renders a form request with labeled fields and required markers", () => {
const html = renderCard({
request: {
...request,
version: 2,
request_id: "clarification:call-form",
question: "Please provide the expense details.",
input_mode: "form",
options: undefined,
fields: [
{ name: "amount", label: "Amount", type: "number", required: true },
{
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 },
{
name: "design",
label: "DesignPriority",
type: "checkbox",
required: false,
},
],
},
});
expect(html).toContain("Amount");
expect(html).toContain("*");
expect(html).toContain("Receipts");
expect(html).toContain("A-1");
expect(html).toContain("Note");
expect(html).toContain("Submit");
// Checkbox fields render as a single toggle row — the label must not be
// duplicated by an additional field label above the control.
expect(html.split("DesignPriority").length - 1).toBe(1);
});
it("associates form labels with controls and marks required fields", () => {
const html = renderCard({
request: {
...request,
version: 2,
request_id: "clarification:call-form",
question: "Please provide the expense details.",
input_mode: "form",
options: undefined,
fields: [
{ name: "amount", label: "Amount", type: "number", required: true },
{ name: "note", label: "Note", type: "textarea", required: false },
],
},
});
// Every visible label is linked to its control via htmlFor/id.
const htmlForIds = [...html.matchAll(/<label[^>]*for="([^"]+)"/g)].map(
(match) => match[1],
);
expect(htmlForIds.length).toBe(2);
for (const id of htmlForIds) {
expect(html).toContain(`id="${id}"`);
}
expect(html).toContain('aria-required="true"');
});
it("findMissingRequiredFields flags empty required values only", () => {
const fields = [
{
name: "amount",
label: "Amount",
type: "number" as const,
required: true,
},
{
name: "note",
label: "Note",
type: "text" as const,
required: false,
},
{
name: "receipts",
label: "Receipts",
type: "multi_select" as const,
required: true,
options: [{ id: "o1", label: "A-1", value: "A-1" }],
},
];
expect(
findMissingRequiredFields(fields, {}).map((field) => field.name),
).toEqual(["amount", "receipts"]);
expect(
findMissingRequiredFields(fields, {
amount: " ",
receipts: [],
}).map((field) => field.name),
).toEqual(["amount", "receipts"]);
expect(
findMissingRequiredFields(fields, {
amount: "300",
receipts: ["A-1"],
}),
).toEqual([]);
});
it("does not submit text with Enter while IME composition is active", () => {
expect(shouldSubmitHumanInputTextOnKeyDown(keyEvent())).toBe(true);
expect(
shouldSubmitHumanInputTextOnKeyDown(keyEvent({ shiftKey: true })),
).toBe(false);
expect(
shouldSubmitHumanInputTextOnKeyDown(keyEvent({ isComposing: true })),
).toBe(false);
expect(
shouldSubmitHumanInputTextOnKeyDown(keyEvent({ keyCode: 229 })),
).toBe(false);
expect(shouldSubmitHumanInputTextOnKeyDown(keyEvent(), true)).toBe(false);
});
});
function renderCard(props: Partial<Parameters<typeof HumanInputCard>[0]> = {}) {
return renderToStaticMarkup(
createElement(
I18nContext.Provider,
{
value: {
locale: "en-US",
setLocale: () => undefined,
},
},
createElement(HumanInputCard, {
request,
onSubmit: () => undefined,
...props,
}),
),
);
}
function keyEvent({
isComposing = false,
key = "Enter",
keyCode = 13,
shiftKey = false,
}: {
isComposing?: boolean;
key?: string;
keyCode?: number;
shiftKey?: boolean;
} = {}) {
return {
key,
keyCode,
nativeEvent: { isComposing },
shiftKey,
} as unknown as KeyboardEvent<HTMLTextAreaElement>;
}