From 1baa8ad69609530b95057b581f69ced487a5ed9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=98=BF=E6=B3=BD?= Date: Mon, 27 Jul 2026 14:05:31 +0800 Subject: [PATCH] feat(clarification): structured form fields for human-input cards (#4400 Phase 1) (#4406) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * 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 * 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 * 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 * fix(frontend): keep clarification selects controlled --------- Co-authored-by: Claude Fable 5 --- backend/AGENTS.md | 4 +- .../middlewares/clarification_middleware.py | 211 +++++++++- .../tools/builtins/clarification_tool.py | 41 +- .../tests/test_clarification_middleware.py | 390 ++++++++++++++++++ backend/tests/test_human_input.py | 7 + frontend/AGENTS.md | 2 +- .../workspace/messages/human-input-card.tsx | 348 +++++++++++++++- frontend/src/core/i18n/locales/en-US.ts | 3 + frontend/src/core/i18n/locales/types.ts | 3 + frontend/src/core/i18n/locales/zh-CN.ts | 3 + frontend/src/core/messages/human-input.ts | 280 ++++++++++++- .../messages/human-input-card.dom.test.tsx | 103 +++++ .../messages/human-input-card.test.ts | 111 +++++ .../unit/core/messages/human-input.test.ts | 348 ++++++++++++++++ 14 files changed, 1826 insertions(+), 28 deletions(-) create mode 100644 frontend/tests/unit/components/workspace/messages/human-input-card.dom.test.tsx diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 79948b0f3..7960776ed 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -365,7 +365,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc 32. **TerminalResponseMiddleware** - When a provider returns an empty terminal `AIMessage` after tool execution, injects a hidden recovery prompt and retries the model once; a second empty response is replaced in checkpoint state by a visible error fallback marked for the run worker, so the run finishes as an error instead of a silent success 33. **ModelLengthFinishReasonMiddleware** - Records `stop_reason=model_length_capped` when provider-specific length detectors match a terminal `AIMessage` without tool-call intent (`finish_reason=length` / `MAX_TOKENS`, or `stop_reason=max_tokens`), preserving the original assistant content and never reparsing textual tool-call-like envelopes 34. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Suppresses tool execution when the provider safety-terminated the response (e.g. `finish_reason=content_filter`); registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first -35. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context. +35. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). Payloads are versioned: legacy modes (`free_text` / `choice_with_other`) keep `version: 1` unchanged, while the v2 `form` mode (from `fields`) carries `version: 2` so older frontends reject the payload and degrade to the plain-text fallback. Field normalization is deterministic and lives in the middleware, not the tool schema — the middleware short-circuits before tool execution, so tool-arg typing alone provides no runtime validation. Validation is atomic: any structurally broken entry (non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member like `__proto__`/`constructor`, exceeding the caps of 16 fields / 24 options per field / 200 chars per text, or the whole normalized definition exceeding `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8 — the per-item caps alone admit forms whose IM text fallback would blow channel delivery limits and truncate away trailing fields) degrades the whole form to the legacy option/free-text modes, so a card can never render "complete" while silently missing a business field; benign issues keep local degradation (unknown types — including unhashable JSON like `type: []`, which must never raise from the membership probe — and option-less selects become `text`), and options are trimmed/deduped with blanks dropped (both form-level and top-level) because the frontend parser rejects blank option labels. Checkbox fields are booleans that default to an explicit "no"; `required` on a checkbox means must-agree/consent semantics. The response protocol is deliberately unchanged (v1 `text`/`option` only): form cards submit a readable text summary as `response_kind: "text"`, so journal persistence and answered-card recovery need no new allowlist entries. Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context. ### Configuration System @@ -613,7 +613,7 @@ copying a raw checkpoint because delta state is not self-contained in one tuple. 2. **MCP tools** - From enabled MCP servers (lazy initialized, cached with resolved-path + content-signature invalidation) 3. **Built-in tools**: - `present_files` - Make output files visible to user (only `/mnt/user-data/outputs`) - - `ask_clarification` - Request clarification (intercepted by ClarificationMiddleware, which preserves text fallback and adds `artifact.human_input` for Web UI Human Input Cards) + - `ask_clarification` - Request clarification (intercepted by ClarificationMiddleware, which preserves text fallback and adds `artifact.human_input` for Web UI Human Input Cards). Beyond free text and single choice, the request-side v2 protocol supports `fields` (structured form card collecting several values at once; field types: text/textarea/number/select/multi_select/checkbox/date, validated and normalized server-side in the middleware — invalid entries are dropped, unknown types degrade to `text`; a standalone multi-select question is a one-field form). Replies stay on the v1 response protocol (`text`/`option`): the form card submits a readable text summary - `view_image` - Read image as base64 (added only if model supports vision) - `setup_agent` - Bootstrap-only: persist a brand-new custom agent's `SOUL.md` and `config.yaml`. Bound only when `is_bootstrap=True`. - `update_agent` - Custom-agent-only: persist self-updates to the current agent's `SOUL.md` / `config.yaml` from inside a normal chat (partial update + atomic write). Bound when `agent_name` is set and `is_bootstrap=False`. diff --git a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py index 3adf0d9c4..488740331 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py @@ -15,6 +15,47 @@ from langgraph.types import Command logger = logging.getLogger(__name__) +# Whitelisted form field types; anything else degrades to "text" so a bad +# model-provided type can never produce an unrenderable card. +FORM_FIELD_TYPES = frozenset({"text", "textarea", "number", "select", "multi_select", "checkbox", "date"}) +_OPTION_FIELD_TYPES = frozenset({"select", "multi_select"}) + +# Field names that collide with JavaScript Object.prototype properties. The +# frontend stores form values in a plain object keyed by field name, so these +# would read inherited prototype members instead of user input. +_RESERVED_FIELD_NAMES = frozenset( + { + "__proto__", + "constructor", + "prototype", + "toString", + "toLocaleString", + "valueOf", + "hasOwnProperty", + "isPrototypeOf", + "propertyIsEnumerable", + "__defineGetter__", + "__defineSetter__", + "__lookupGetter__", + "__lookupSetter__", + } +) + +# Hard caps so a runaway model cannot publish an unbounded form. Exceeding a +# cap is a structural error: the whole form degrades to the legacy modes +# instead of silently truncating business fields. +MAX_FORM_FIELDS = 16 +MAX_FIELD_OPTIONS = 24 +MAX_FIELD_TEXT_CHARS = 200 +# Total budget over the serialized normalized fields, in UTF-8 bytes. The +# per-item caps alone still admit forms whose plain-text IM fallback exceeds +# channel delivery limits (Slack truncates at 40k chars per message; Feishu +# guides ~30KB per card), which would silently drop trailing fields — the very +# thing atomic validation exists to prevent. 16KB keeps the fallback text of +# any accepted form comfortably inside the strictest supported channel while +# leaving headroom for question/context. +MAX_FORM_SERIALIZED_BYTES = 16_384 + class ClarificationMiddlewareState(AgentState): """Compatible with the `ThreadState` schema.""" @@ -62,21 +103,141 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): if not isinstance(options, list): options = [options] - return [str(option) for option in options] + # Trim, drop blanks, and dedupe (order-preserving): the frontend parser + # rejects the whole payload on blank option labels, so they must never + # be emitted. + normalized: list[str] = [] + seen: set[str] = set() + for option in options: + text = str(option).strip() + if not text or text in seen: + continue + seen.add(text) + normalized.append(text) + return normalized - def _build_human_input_payload(self, args: dict[str, Any], *, tool_call_id: str, request_id: str) -> dict[str, Any]: - """Build the structured UI payload while keeping ToolMessage.content as fallback.""" + @staticmethod + def _normalize_bool(raw: Any) -> bool: + """Coerce a model-provided boolean; some models serialize booleans as strings or 1/0.""" + if isinstance(raw, bool): + return raw + if isinstance(raw, int | float): + return bool(raw) + if isinstance(raw, str): + return raw.strip().lower() == "true" + return False + + def _normalize_fields(self, raw_fields: Any) -> list[dict[str, Any]]: + """Normalize tool-provided form fields into the validated v2 field schema. + + Validation is atomic: any structurally broken entry (non-dict, bad or + reserved or duplicate name, over-cap counts/lengths) invalidates the + whole form so the card can never render "complete" while silently + missing a required business field. Benign issues keep their local + degradation: unknown types and option-less selects become ``text``. + """ + fields = raw_fields + if isinstance(fields, str): + try: + fields = json.loads(fields) + except (json.JSONDecodeError, TypeError): + return [] + if not isinstance(fields, list): + return [] + if len(fields) > MAX_FORM_FIELDS: + return [] + + normalized: list[dict[str, Any]] = [] + seen_names: set[str] = set() + for entry in fields: + if not isinstance(entry, dict): + return [] + raw_name = entry.get("name") + if not isinstance(raw_name, str) or not raw_name.strip(): + return [] + name = raw_name.strip() + if name in _RESERVED_FIELD_NAMES or name in seen_names or len(name) > MAX_FIELD_TEXT_CHARS: + return [] + seen_names.add(name) + + raw_label = entry.get("label") + label = raw_label.strip() if isinstance(raw_label, str) and raw_label.strip() else name + if len(label) > MAX_FIELD_TEXT_CHARS: + return [] + + field_type = entry.get("type") + # isinstance guard first: `type: []` / `type: {}` are legal JSON + # from a model, and an unhashable membership probe would raise + # TypeError instead of degrading. + if not isinstance(field_type, str) or field_type not in FORM_FIELD_TYPES: + field_type = "text" + + options = self._normalize_options(entry.get("options")) if field_type in _OPTION_FIELD_TYPES else [] + if len(options) > MAX_FIELD_OPTIONS or any(len(option) > MAX_FIELD_TEXT_CHARS for option in options): + return [] + if field_type in _OPTION_FIELD_TYPES and not options: + field_type = "text" + + field: dict[str, Any] = { + "name": name, + "label": label, + "type": field_type, + "required": self._normalize_bool(entry.get("required")), + } + if field_type in _OPTION_FIELD_TYPES: + field["options"] = [ + { + "id": f"{name}-option-{index}", + "label": option, + "value": option, + } + for index, option in enumerate(options, 1) + ] + placeholder = entry.get("placeholder") + if isinstance(placeholder, str) and placeholder.strip(): + if len(placeholder.strip()) > MAX_FIELD_TEXT_CHARS: + return [] + field["placeholder"] = placeholder.strip() + normalized.append(field) + + if len(json.dumps(normalized, ensure_ascii=False).encode("utf-8")) > MAX_FORM_SERIALIZED_BYTES: + return [] + + return normalized + + def _build_human_input_payload(self, args: dict[str, Any], *, tool_call_id: str, request_id: str, fields: list[dict[str, Any]] | None = None) -> dict[str, Any]: + """Build the structured UI payload while keeping ToolMessage.content as fallback. + + Protocol versioning: legacy modes (``free_text`` / ``choice_with_other``) + keep ``version: 1`` so their wire format is unchanged; the v2 ``form`` + mode carries ``version: 2`` so older frontends reject the payload and + degrade to the plain-text ToolMessage content. Replies stay on the v1 + response protocol (``text`` / ``option``) — the form card submits a + readable ``value`` summary, so no new response kind is introduced. + + ``fields`` accepts an already-normalized list so callers rendering both + the payload and the text fallback normalize only once. + """ + if fields is None: + fields = self._normalize_fields(args.get("fields")) options = self._normalize_options(args.get("options", [])) clarification_type = str(args.get("clarification_type", "missing_info")) + if fields: + version, input_mode = 2, "form" + elif options: + version, input_mode = 1, "choice_with_other" + else: + version, input_mode = 1, "free_text" + payload: dict[str, Any] = { - "version": 1, + "version": version, "kind": "human_input_request", "source": "ask_clarification", "request_id": request_id, "clarification_type": clarification_type, "question": str(args.get("question") or ""), - "input_mode": "choice_with_other" if options else "free_text", + "input_mode": input_mode, } if tool_call_id: @@ -86,7 +247,9 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): context = args.get("context") payload["context"] = None if context is None else str(context) - if options: + if input_mode == "form": + payload["fields"] = fields + elif options: payload["options"] = [ { "id": f"option-{index}", @@ -109,18 +272,24 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): """ return any("\u4e00" <= char <= "\u9fff" for char in text) - def _format_clarification_message(self, args: dict) -> str: + def _format_clarification_message(self, args: dict, fields: list[dict[str, Any]] | None = None) -> str: """Format the clarification arguments into a user-friendly message. Args: args: The tool call arguments containing clarification details + fields: Already-normalized form fields, so callers rendering both + the payload and this fallback normalize only once Returns: Formatted message string """ question = args.get("question", "") - clarification_type = args.get("clarification_type", "missing_info") + # str() coercion keeps the icon lookup hashable — `clarification_type: + # []` is legal JSON from a model and would raise TypeError as a dict key. + clarification_type = str(args.get("clarification_type", "missing_info")) context = args.get("context") + if fields is None: + fields = self._normalize_fields(args.get("fields")) options = self._normalize_options(args.get("options", [])) # Type-specific icons @@ -146,8 +315,22 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): # Just the question with icon message_parts.append(f"{icon} {question}") - # Add options in a cleaner format - if options and len(options) > 0: + # Form fields take precedence over options, mirroring the payload logic. + if fields: + message_parts.append("") # blank line for spacing + for i, field in enumerate(fields, 1): + line = f" {i}. {field['label']}" + if field["required"]: + line += " (required)" + field_options = field.get("options") + if field_options: + line += " — options: " + " / ".join(option["label"] for option in field_options) + if field["type"] == "multi_select": + line += " (multiple allowed)" + message_parts.append(line) + message_parts.append("") + message_parts.append("Please reply with a value for each field.") + elif options and len(options) > 0: message_parts.append("") # blank line for spacing for i, option in enumerate(options, 1): message_parts.append(f" {i}. {option}") @@ -208,14 +391,18 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): logger.info("Intercepted clarification request") logger.debug("Clarification question: %s", question) + # Normalize form fields once; both the text fallback and the payload + # consume the same result. + fields = self._normalize_fields(args.get("fields")) + # Format the clarification message - formatted_message = self._format_clarification_message(args) + formatted_message = self._format_clarification_message(args, fields=fields) # Get the tool call ID tool_call_id = request.tool_call.get("id", "") request_id = self._stable_message_id(tool_call_id, formatted_message) - human_input_payload = self._build_human_input_payload(args, tool_call_id=tool_call_id, request_id=request_id) + human_input_payload = self._build_human_input_payload(args, tool_call_id=tool_call_id, request_id=request_id, fields=fields) # Create a ToolMessage with the formatted question # This will be added to the message history diff --git a/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py b/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py index 49c3db102..8bf0de9b4 100644 --- a/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py @@ -1,8 +1,24 @@ -from typing import Literal +from typing import Literal, Required, TypedDict from langchain.tools import tool +class ClarificationFormField(TypedDict, total=False): + """One form field definition for a structured clarification card. + + The model-visible schema documents the item shape; runtime validation + still happens defensively in ``ClarificationMiddleware`` because the + middleware intercepts the call before tool execution. + """ + + name: Required[str] + label: str + type: Literal["text", "textarea", "number", "select", "multi_select", "checkbox", "date"] + required: bool + options: list[str] + placeholder: str + + @tool("ask_clarification", parse_docstring=True, return_direct=True) def ask_clarification_tool( question: str, @@ -15,6 +31,7 @@ def ask_clarification_tool( ], context: str | None = None, options: list[str] | None = None, + fields: list[ClarificationFormField] | None = None, ) -> str: """Ask the user for clarification when you need more information to proceed. @@ -36,11 +53,22 @@ def ask_clarification_tool( - You're about to perform a potentially dangerous operation - You have a recommendation but need user approval + Choosing the interaction shape: + - One open question -> just `question` (free text input) + - Pick exactly one option -> `options` + - Pick several options -> a single `fields` entry of type `multi_select` + - Collect several values at once (e.g. a set of parameters for one action) -> + `fields`, which renders a single structured form instead of several + sequential questions. Prefer one form over asking field-by-field. + Best practices: - - Ask ONE clarification at a time for clarity + - Ask ONE clarification at a time for clarity; a form with several fields + still counts as one clarification - Be specific and clear in your question - Don't make assumptions when clarification is needed - For risky operations, ALWAYS ask for confirmation + - If a skill provides a predefined field template, pass it through `fields` + unchanged instead of redesigning it - After calling this tool, execution will be interrupted automatically Args: @@ -48,6 +76,15 @@ def ask_clarification_tool( clarification_type: The type of clarification needed (missing_info, ambiguous_requirement, approach_choice, risk_confirmation, suggestion). context: Optional context explaining why clarification is needed. Helps the user understand the situation. options: Optional list of choices (for approach_choice or suggestion types). Present clear options for the user to choose from. + fields: Optional form field definitions for collecting multiple values in one card; takes precedence over `options`. + Each field is an object with `name` (unique identifier, required; avoid JavaScript prototype names like + `constructor` or `toString`), `label` (display text, defaults to name), `type` (one of: text, textarea, + number, select, multi_select, checkbox, date; defaults to text), `required` (boolean, defaults to false), + `options` (list of strings, required for select/multi_select types), and `placeholder` (optional hint text). + A `checkbox` field is a boolean that defaults to "no"; set `required` on a checkbox only for + must-agree/consent semantics (the user has to tick it to submit). Keep forms bounded: at most 16 fields, + 24 options per field, and 200 characters per name/label/option/placeholder — exceeding a limit degrades + the whole request to a plain-text question. """ # This is a placeholder implementation # The actual logic is handled by ClarificationMiddleware which intercepts this tool call diff --git a/backend/tests/test_clarification_middleware.py b/backend/tests/test_clarification_middleware.py index 040081e12..8155443f5 100644 --- a/backend/tests/test_clarification_middleware.py +++ b/backend/tests/test_clarification_middleware.py @@ -215,6 +215,396 @@ class TestHumanInputPayload: assert "options" not in payload +class TestFormPayload: + """v2 protocol: fields render a structured form card.""" + + def _fields(self): + return [ + {"name": "amount", "label": "Amount", "type": "number", "required": True}, + {"name": "category", "label": "Category", "type": "select", "options": ["travel", "meals"], "required": True}, + {"name": "receipts", "label": "Receipts", "type": "multi_select", "options": ["A-1", "A-2"]}, + {"name": "note", "type": "textarea"}, + ] + + def test_form_payload_with_native_fields(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Please provide the expense details.", + "clarification_type": "missing_info", + "fields": self._fields(), + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["version"] == 2 + assert payload["input_mode"] == "form" + assert payload["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}, + ] + + def test_form_payload_with_json_string_fields(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": json.dumps([{"name": "amount", "type": "number"}]), + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "form" + assert payload["fields"] == [{"name": "amount", "label": "amount", "type": "number", "required": False}] + + def test_form_takes_precedence_over_options(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "amount"}], + "options": ["dev", "prod"], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "form" + assert "options" not in payload + + def test_unknown_field_type_degrades_to_text(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "amount", "type": "slider"}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["fields"][0]["type"] == "text" + + def test_select_without_options_degrades_to_text(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "category", "type": "select"}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["fields"][0]["type"] == "text" + assert "options" not in payload["fields"][0] + + def test_any_invalid_field_entry_degrades_whole_form(self, middleware): + """Atomic validation: a structurally broken entry must not silently + vanish from an otherwise-rendered form — the whole form degrades.""" + for bad_entry in ["not-a-dict", {"label": "no name"}, {"name": " "}]: + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [bad_entry, {"name": "amount", "type": "number"}], + "options": ["dev", "prod"], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["version"] == 1 + assert payload["input_mode"] == "choice_with_other" + assert "fields" not in payload + + def test_duplicate_field_names_degrade_whole_form(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [ + {"name": "amount", "type": "number"}, + {"name": "amount", "type": "text"}, + ], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["version"] == 1 + assert payload["input_mode"] == "free_text" + assert "fields" not in payload + + def test_reserved_prototype_field_names_degrade_whole_form(self, middleware): + """`__proto__`/`constructor`/... collide with JS Object.prototype in the + frontend form-value store and must be rejected server-side.""" + for reserved in ["__proto__", "constructor", "toString", "hasOwnProperty"]: + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": reserved, "type": "text"}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "free_text", reserved + assert "fields" not in payload + + def test_field_options_are_trimmed_deduped_and_blank_dropped(self, middleware): + """Backend must never emit blank option labels — the frontend parser + rejects the whole payload on them.""" + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "category", "type": "select", "options": [" travel ", "", " ", "travel", "meals"]}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert [option["label"] for option in payload["fields"][0]["options"]] == ["travel", "meals"] + + def test_field_count_over_cap_degrades_whole_form(self, middleware): + fields = [{"name": f"field_{i}", "type": "text"} for i in range(17)] + payload = middleware._build_human_input_payload( + {"question": "Details please", "clarification_type": "missing_info", "fields": fields}, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "free_text" + assert "fields" not in payload + + def test_option_count_over_cap_degrades_whole_form(self, middleware): + options = [f"option-{i}" for i in range(25)] + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "category", "type": "select", "options": options}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "free_text" + assert "fields" not in payload + + def test_overlong_field_text_degrades_whole_form(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "amount", "label": "x" * 201, "type": "number"}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "free_text" + assert "fields" not in payload + + def test_top_level_options_are_trimmed_and_blank_dropped(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Which env?", + "clarification_type": "approach_choice", + "options": [" dev ", "", "prod", "dev"], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["options"] == [ + {"id": "option-1", "label": "dev", "value": "dev"}, + {"id": "option-2", "label": "prod", "value": "prod"}, + ] + + def test_unhashable_field_type_degrades_to_text_not_crash(self, middleware): + """`type: []` / `type: {}` are legal JSON from a model; an unhashable + membership check would raise TypeError and (with return_direct=True) + end the turn with an error instead of any card or fallback.""" + for bad_type in [[], {}, ["select"], {"t": "select"}]: + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "amount", "type": bad_type}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["fields"][0]["type"] == "text" + + def test_unhashable_clarification_type_does_not_crash(self, middleware): + """Same crash class as unhashable field types: `clarification_type: []` + must not raise in the icon lookup or payload builder.""" + from langgraph.types import Command + + request = SimpleNamespace( + tool_call={ + "name": "ask_clarification", + "id": "call-clarify-1", + "args": {"question": "q?", "clarification_type": [], "fields": [{"name": "amount", "type": []}]}, + }, + runtime=None, + ) + + result = middleware.wrap_tool_call(request, lambda _req: pytest.fail("handler should not be called")) + + assert isinstance(result, Command) + message = result.update["messages"][0] + assert message.artifact["human_input"]["input_mode"] == "form" + + def test_serialized_fields_over_byte_budget_degrade_whole_form(self, middleware): + """Per-item caps alone allow a form whose IM text fallback exceeds + channel limits (Slack 40k chars, Feishu ~30KB card); a total serialized + byte budget must bound the whole definition.""" + fields = [ + { + "name": f"field_{i}", + "label": "标" * 190, + "type": "select", + "options": ["选" * 190 + str(j) for j in range(20)], + } + for i in range(16) + ] + payload = middleware._build_human_input_payload( + {"question": "Details please", "clarification_type": "missing_info", "fields": fields}, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["input_mode"] == "free_text" + assert "fields" not in payload + + def test_accepted_forms_keep_text_fallback_under_channel_limits(self, middleware): + """Boundary: any form the byte budget accepts must produce an IM text + fallback deliverable to the strictest supported channel (~30KB).""" + # Grow a form until the budget rejects it; the largest accepted form's + # fallback must stay under the channel bound. + largest_accepted_fallback = None + for count in range(1, 17): + fields = [ + { + "name": f"field_{i}", + "label": "标" * 150, + "type": "select", + "options": ["选" * 100 + str(j) for j in range(10)], + } + for i in range(count) + ] + args = {"question": "Q", "clarification_type": "missing_info", "fields": fields} + payload = middleware._build_human_input_payload(args, tool_call_id="c", request_id="clarification:c") + if payload["input_mode"] != "form": + break + largest_accepted_fallback = middleware._format_clarification_message(args) + + assert largest_accepted_fallback is not None + assert len(largest_accepted_fallback.encode("utf-8")) < 30_000 + + def test_required_accepts_integer_serialization(self, middleware): + """Some providers emit 1/0 for booleans; `required: 1` must not + silently flip a model-intended required field to optional.""" + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [ + {"name": "amount", "type": "number", "required": 1}, + {"name": "note", "type": "text", "required": 0}, + ], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["fields"][0]["required"] is True + assert payload["fields"][1]["required"] is False + + def test_field_placeholder_is_preserved(self, middleware): + payload = middleware._build_human_input_payload( + { + "question": "Details please", + "clarification_type": "missing_info", + "fields": [{"name": "note", "placeholder": "Optional remarks"}], + }, + tool_call_id="call-abc", + request_id="clarification:call-abc", + ) + + assert payload["fields"][0]["placeholder"] == "Optional remarks" + + def test_form_fallback_text_lists_fields(self, middleware): + result = middleware._format_clarification_message( + { + "question": "Please provide the expense details.", + "clarification_type": "missing_info", + "fields": self._fields(), + } + ) + + assert "1. Amount (required)" in result + assert "2. Category (required) — options: travel / meals" in result + assert "3. Receipts — options: A-1 / A-2 (multiple allowed)" in result + assert "4. note" in result + assert "Please reply with a value for each field." in result + + +class TestClarificationToolSchema: + """The tool schema must expose the v2 form parameter (request-side only).""" + + def test_tool_exposes_fields_argument(self): + from deerflow.tools.builtins.clarification_tool import ask_clarification_tool + + schema = ask_clarification_tool.args + assert "fields" in schema + # The reply protocol stays v1 (text/option): no top-level multi_select + # mode — a standalone multi-select question is a one-field form. + assert "multi_select" not in schema + + def test_fields_item_schema_is_typed(self): + """The provider-facing schema must expose the field item shape (typed + via ClarificationFormField), not an opaque object relying on the + docstring alone.""" + from langchain_core.utils.function_calling import convert_to_openai_tool + + from deerflow.tools.builtins.clarification_tool import ask_clarification_tool + + parameters = convert_to_openai_tool(ask_clarification_tool)["function"]["parameters"] + items = parameters["properties"]["fields"]["anyOf"][0]["items"] + + assert items["required"] == ["name"] + assert sorted(items["properties"].keys()) == ["label", "name", "options", "placeholder", "required", "type"] + assert items["properties"]["type"]["enum"] == ["text", "textarea", "number", "select", "multi_select", "checkbox", "date"] + + class TestClarificationCommandIdempotency: """Clarification tool-call retries should not duplicate messages in state.""" diff --git a/backend/tests/test_human_input.py b/backend/tests/test_human_input.py index 999e8cd5c..d8626cb63 100644 --- a/backend/tests/test_human_input.py +++ b/backend/tests/test_human_input.py @@ -22,3 +22,10 @@ def test_read_human_input_response_preserves_non_empty_value(): assert response is not None assert response["value"] == " staging " + + +def test_read_human_input_response_rejects_unknown_kind(): + raw = _text_response("staging") + raw["response_kind"] = "unknown" + + assert read_human_input_response({"human_input_response": raw}) is None diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 9906415b1..0b28fcf94 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -81,7 +81,7 @@ Auth UI note: the login page's "keep me signed in" option submits only `remember `/goal` and `/compact` are built-in composer commands, not skill activations. `src/components/workspace/input-box.tsx` intercepts `/goal`, `/goal clear`, and `/goal ` before normal chat submission, calling Gateway `GET/PUT/DELETE /api/threads/{thread_id}/goal`. Setting `/goal ` also submits the condition text as the next user task so the agent starts running immediately; status and clear do not start a run. Goal and compact requests are tied to the current `threadId` with an `AbortController`, so switching threads or unmounting the composer aborts in-flight requests and stale responses cannot update the new thread's composer state. The chat pages render `GoalStatus` above the composer from `AgentThreadState.goal`, with local optimistic state until the next stream `values` update arrives. `/compact` calls `POST /api/threads/{thread_id}/compact` to summarize older active context while leaving the full visible chat history intact; it is skipped on new/empty threads and blocked server-side while a run is in flight. Thread rename uses the same serialized state-write route; the rename dialog stays open and surfaces the server error when an active run returns 409. -Human input requests are a structured message protocol layered on normal chat history. The backend writes request payloads to `ToolMessage.artifact.human_input`, `src/core/messages/human-input.ts` owns the runtime validators/types, and `src/components/workspace/messages/human-input-card.tsx` renders the reusable card. `MessageList` owns answered/latest/pending state for visible cards, but derives answered responses from raw `thread.messages` because replies are hidden; pending cards clear when the hidden reply appears, when dispatch is dropped, or when a new `thread.error` reports an async stream failure. Page-level submit callbacks must send a normal human message and put `hide_from_ui: true` plus the response payload in the fourth `sendMessage(..., options)` argument as `options.additionalKwargs`; the third argument remains run context such as `{ agent_name }`. Composer entry points should disable normal bottom input while `hasOpenHumanInputRequest(...)` is true so users answer through the card and preserve response metadata. +Human input requests are a structured message protocol layered on normal chat history. The backend writes request payloads to `ToolMessage.artifact.human_input`, `src/core/messages/human-input.ts` owns the runtime validators/types, and `src/components/workspace/messages/human-input-card.tsx` renders the reusable card. The protocol is versioned on the request side only: v1 covers `free_text` / `choice_with_other`, and v2 adds `form` (typed fields — text/textarea/number/select/multi_select/checkbox/date — with required-field validation in the card). Replies deliberately stay on the v1 response protocol: the form card submits a `response_kind: "text"` reply whose value is the human-readable summary plus one JSON block keyed by stable field names (`buildHumanInputFormSubmissionValue` — the readable part alone is ambiguous because labels/values may contain the separators), so the model can reconstruct the submitted mapping without a structured response kind. The validators reject unknown versions/modes (and field names colliding with JS `Object.prototype` members) so future protocol bumps degrade to the plain-text ToolMessage fallback rather than rendering a broken card. Form values are read through own-property access only (`readHumanInputFormValue`); select fields stay controlled from their empty-string placeholder state through selection; checkbox fields are native `` controls seeded to an explicit `false` (`buildInitialHumanInputFormValues`) so an untouched checkbox submits as "no" while a `required` checkbox keeps must-agree semantics (no HTML `required` attribute — native constraint validation would intercept the custom submit path), and form controls carry label/`htmlFor`, `aria-required` plus a visually-hidden localized "required" marker, and `aria-invalid`/error associations whose error node stays mounted while any field is still invalid. Legacy-fallback closure: `deriveHumanInputThreadState` treats a visible plain human message as answering the latest unanswered request opened before it (only the latest — nothing guarantees a single outstanding request across runs, and closing all would silently swallow older decisions; an older request left open simply becomes the active card again) — an old v1-only frontend degrades a v2 request to text and the user replies through the normal composer without response metadata, and without this rule an upgraded frontend would see that request as still open and lock the composer. `MessageList` owns answered/latest/pending state for visible cards, but derives answered responses from raw `thread.messages` because replies are hidden; pending cards clear when the hidden reply appears, when dispatch is dropped, or when a new `thread.error` reports an async stream failure. Page-level submit callbacks must send a normal human message and put `hide_from_ui: true` plus the response payload in the fourth `sendMessage(..., options)` argument as `options.additionalKwargs`; the third argument remains run context such as `{ agent_name }`. Composer entry points should disable normal bottom input while `hasOpenHumanInputRequest(...)` is true so users answer through the card and preserve response metadata. Tool-calling AI messages can contain user-visible text as well as `tool_calls`. `core/messages/utils.ts` keeps these turns in an `assistant:processing` group, and `components/workspace/messages/message-group.tsx` must render the visible text as a processing step instead of treating the message as only tool metadata. This preserves provider text such as error explanations or "trying another approach" notes during tool-heavy runs. diff --git a/frontend/src/components/workspace/messages/human-input-card.tsx b/frontend/src/components/workspace/messages/human-input-card.tsx index e4d268752..aaad8e264 100644 --- a/frontend/src/components/workspace/messages/human-input-card.tsx +++ b/frontend/src/components/workspace/messages/human-input-card.tsx @@ -2,6 +2,7 @@ import { CheckCircle2Icon, + CheckIcon, Loader2Icon, MessageCircleQuestionMarkIcon, } from "lucide-react"; @@ -9,11 +10,25 @@ import { useId, useState, type KeyboardEvent } from "react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; import { Textarea } from "@/components/ui/textarea"; import { useI18n } from "@/core/i18n/hooks"; import { + buildHumanInputFormSubmissionValue, + buildHumanInputFormSummary, + buildInitialHumanInputFormValues, createHumanInputOptionResponse, createHumanInputTextResponse, + readHumanInputFormValue, + type HumanInputField, + type HumanInputFormValue, type HumanInputOption, type HumanInputRequest, type HumanInputResponse, @@ -36,6 +51,169 @@ export function shouldSubmitHumanInputTextOnKeyDown( ); } +function isEmptyFieldValue(value: HumanInputFormValue | undefined) { + if (value === undefined) { + return true; + } + if (typeof value === "string") { + return value.trim().length === 0; + } + if (Array.isArray(value)) { + return value.length === 0; + } + return value === false; +} + +export function findMissingRequiredFields( + fields: HumanInputField[], + values: Record, +) { + // Own-property reads only: field names like "toString" must never resolve + // to inherited Object.prototype members and satisfy required validation. + return fields.filter( + (field) => + field.required && + isEmptyFieldValue(readHumanInputFormValue(values, field.name)), + ); +} + +function FormFieldInput({ + field, + value, + disabled, + selectPlaceholder, + controlId, + labelId, + invalid, + errorId, + onChange, +}: { + field: HumanInputField; + value: HumanInputFormValue | undefined; + disabled: boolean; + selectPlaceholder: string; + controlId: string; + labelId: string; + invalid: boolean; + errorId?: string; + onChange: (value: HumanInputFormValue) => void; +}) { + const stringValue = typeof value === "string" ? value : ""; + const ariaProps = { + "aria-required": field.required || undefined, + "aria-invalid": invalid || undefined, + "aria-describedby": invalid ? errorId : undefined, + }; + + if (field.type === "textarea") { + return ( +