From 4b4a8909c1d3f54c7dbde0b36b3de2a4849bc480 Mon Sep 17 00:00:00 2001 From: GeneAI Date: Thu, 20 Aug 2026 07:42:04 -0400 Subject: [PATCH 1/2] fix: reject unknown DEFINITION keys in form_from_dict + close both MCP schemas (confirmation-pass-1 chair ruling) Chair ruling 2026-08-20 (confirmation-pass-1 ledger): strict rejection, both layers. A typo'd field key ('maximun': 10) silently built a bound-less field that validated any answer clean; #37 covered the answer side only. - form_from_dict names every unrecognized top-level and field-level key ('unknown definition key ...'), aliases label/questions kept - _field_schema/_form_schema declare additionalProperties: false (D3 mirror to attune-ai picks this up at next release-gated re-sync) - parity ratchet test: advertised schema keys == parser key set - regression tests incl. the exact ledger repro; CHANGELOG entry Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 11 ++++++ src/attune_forms/bridge.py | 55 +++++++++++++++++++++++++++-- src/attune_forms/mcp_server.py | 6 ++++ tests/test_bridge.py | 64 ++++++++++++++++++++++++++++++++++ tests/test_mcp_server.py | 22 ++++++++++++ 5 files changed, 156 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2721c88..0c46fcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ follow [SemVer](https://semver.org/). ## [Unreleased] ### Fixed +- Unknown DEFINITION keys are now named problems instead of silently + ignored (confirmation-pass-1 chair ruling, 2026-08-20): a typo'd + field key (`"maximun": 10`) built a bound-less field that validated + any answer clean. `form_from_dict` rejects every unrecognized + top-level and field-level key (`unknown definition key '...'`, + mirroring #37's answer-side wording; the `label`/`questions` aliases + stay accepted), and the MCP `inputSchema` declares + `additionalProperties: false` on both the form and field objects so + the SDK gate agrees with the parser. The schema is D3-mirrored to + attune-ai — the mirror must pick up `additionalProperties` at the + next release-gated re-sync - **Directly-built D2 gate with a `default` can no longer pass unanswered** (checkpoint-2 promoted item, 2026-08-20 — empirically confirmed). The no-`default` rule for the two-way constructs (confirm, diff --git a/src/attune_forms/bridge.py b/src/attune_forms/bridge.py index 8f8392e..d0fe079 100644 --- a/src/attune_forms/bridge.py +++ b/src/attune_forms/bridge.py @@ -796,12 +796,54 @@ def _parse_list_style( return list_style, [] +# The definition-side twin of the #37 answer-side gate: a key the field +# parsers never read is a typo or an unsupported construct, and silently +# ignoring it invents a constraint that does not exist ("maximun": 10 → +# the bound is never built and collect(99999) validates clean). These +# sets must track exactly what form_from_dict and its _parse_* helpers +# read; the parity test against the MCP _field_schema ratchets that. +_DEFINITION_TOP_KEYS = frozenset({"title", "description", "fields", "questions"}) +_DEFINITION_FIELD_KEYS = frozenset( + { + "id", + "text", + "label", + "type", + "options", + "default", + "help_text", + "required", + "minimum", + "maximum", + "max_length", + "rationale", + "recommended", + "option_notes", + "user_position", + "progress_items", + "progress_style", + "endorsements", + "triage_items", + "dispositions", + "suggested", + "consequences", + "list_style", + "inferred_from", + "top_n", + "assumptions", + } +) + + def form_from_dict(data: dict[str, Any]) -> FormSchema: """Build a :class:`FormSchema` from plain serializable data (D3). The declarative artifact a skill / future designer / data source produces. Validates the form *definition* (not answers) and raises - :class:`FormValidationError` listing every problem. + :class:`FormValidationError` listing every problem. A key the + parser does not read — top-level or field-level — is a definition + problem, not ignorable extra data: a typo'd ``"maximun"`` would + otherwise silently drop the bound it meant to declare. Args: data: ``{"title": str, "description"?: str, "fields": [ ... ]}``. @@ -814,7 +856,8 @@ def form_from_dict(data: dict[str, Any]) -> FormSchema: A validated :class:`FormSchema`. Raises: - FormValidationError: If the definition is malformed. + FormValidationError: If the definition is malformed, including + any unknown definition key. """ problems: list[str] = [] @@ -830,6 +873,10 @@ def form_from_dict(data: dict[str, Any]) -> FormSchema: problems.append("form must have a non-empty 'fields' list") raw_fields = [] + for key in data: + if key not in _DEFINITION_TOP_KEYS: + problems.append(f"form has unknown definition key {key!r}") + seen_ids: set[str] = set() questions: list[FormQuestion] = [] for idx, raw in enumerate(raw_fields): @@ -838,6 +885,10 @@ def form_from_dict(data: dict[str, Any]) -> FormSchema: problems.append(f"{where} must be a mapping") continue + for key in raw: + if key not in _DEFINITION_FIELD_KEYS: + problems.append(f"{where} unknown definition key {key!r}") + fid, text, id_problems = _parse_field_identity(where, raw, seen_ids) problems.extend(id_problems) diff --git a/src/attune_forms/mcp_server.py b/src/attune_forms/mcp_server.py index 2900d47..79ce8e9 100644 --- a/src/attune_forms/mcp_server.py +++ b/src/attune_forms/mcp_server.py @@ -159,6 +159,11 @@ def _field_schema() -> dict[str, Any]: "list_style": {"type": "string", "enum": ["ordered", "unordered"]}, }, "required": ["id", "text", "type"], + # Mirrors form_from_dict's strict definition contract: an + # unknown field key is a typo ("maximun") that would silently + # drop the constraint it meant to declare, so the SDK gate + # rejects it rather than waving it through to a lax parse. + "additionalProperties": False, } @@ -172,6 +177,7 @@ def _form_schema() -> dict[str, Any]: "fields": {"type": "array", "items": _field_schema()}, }, "required": ["title", "fields"], + "additionalProperties": False, } diff --git a/tests/test_bridge.py b/tests/test_bridge.py index 92b46fe..37d4d77 100644 --- a/tests/test_bridge.py +++ b/tests/test_bridge.py @@ -366,3 +366,67 @@ def test_dotted_keys_under_present_mapping_stay_exempt(self): {"board": {"One": "keep", "Two": "drop"}, "board.One": "drop"}, ) assert response.responses["board"] == {"One": "keep", "Two": "drop"} + + +class TestUnknownDefinitionKeys: + """Pinned from the confirmation-pass-1 chair ruling (2026-08-20): + the definition-side twin of TestUnknownAnswerKeys. A key the parser + never reads was silently ignored — a typo'd 'maximun' built a + bound-less number field that validated any answer clean. Unknown + top-level and field-level keys are now named definition problems.""" + + def test_ledger_repro_typoed_maximum_is_named(self): + # The exact confirmation-pass repro: the typo'd bound must not + # silently vanish into an unconstrained field. + with pytest.raises( + FormValidationError, match="field\\[0\\] unknown definition key 'maximun'" + ): + form_from_dict( + { + "title": "T", + "fields": [{"id": "n", "text": "N", "type": "number", "maximun": 10}], + } + ) + + def test_every_unknown_field_key_is_named(self): + with pytest.raises(FormValidationError) as exc: + form_from_dict( + { + "title": "T", + "fields": [ + { + "id": "q", + "text": "Q", + "type": "text_input", + "regired": True, + "recomended": "x", + } + ], + } + ) + problems = exc.value.problems + assert "field[0] unknown definition key 'regired'" in problems + assert "field[0] unknown definition key 'recomended'" in problems + + def test_top_level_unknown_key_is_named(self): + with pytest.raises( + FormValidationError, match="form has unknown definition key 'descripton'" + ): + form_from_dict( + { + "title": "T", + "descripton": "typo", + "fields": [{"id": "q", "text": "Q", "type": "text_input"}], + } + ) + + def test_documented_aliases_stay_accepted(self): + # 'label' (for 'text') and 'questions' (for 'fields') are + # documented aliases, not strays. + form = form_from_dict( + { + "title": "T", + "questions": [{"id": "q", "label": "Q", "type": "text_input"}], + } + ) + assert form.questions[0].text == "Q" diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 8eb83e7..83d7892 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -190,3 +190,25 @@ def test_schema_accepts_legal_triage_object_default(): form_from_dict(form) # legal by the library's own validator tools = {t.name: t for t in tool_definitions()} jsonschema.validate({"form": form}, tools["elicitation_render_form"].inputSchema) + + +def test_schemas_close_unknown_keys(): + """Confirmation-pass-1 chair ruling (2026-08-20): unknown definition + keys are strictly rejected at BOTH layers — the advertised schema + must not wave through what form_from_dict names as a problem.""" + from attune_forms.mcp_server import _field_schema, _form_schema + + assert _field_schema()["additionalProperties"] is False + assert _form_schema()["additionalProperties"] is False + + +def test_field_schema_matches_parser_key_set(): + """Ratchet: the mirrored _field_schema and form_from_dict's strict + key set may not drift apart. 'label' is the parser-side alias for + 'text' — never advertised (the schema requires 'text', so the alias + was never usable over stdio).""" + from attune_forms.bridge import _DEFINITION_FIELD_KEYS + from attune_forms.mcp_server import _field_schema + + advertised = set(_field_schema()["properties"]) + assert advertised | {"label"} == set(_DEFINITION_FIELD_KEYS) From f72773c7e41e6af5b70dcb9d0f814157dbef32be Mon Sep 17 00:00:00 2001 From: GeneAI Date: Thu, 20 Aug 2026 09:47:25 -0400 Subject: [PATCH 2/2] style: formatter fix after merge marker-strip (test spacing) --- tests/test_mcp_server.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 767475c..08b6544 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -212,6 +212,8 @@ def test_field_schema_matches_parser_key_set(): advertised = set(_field_schema()["properties"]) assert advertised | {"label"} == set(_DEFINITION_FIELD_KEYS) + + def test_field_schema_covers_the_whole_grammar(): """Drift catcher (architecture review F5, 2026-08-20): the hand-maintained tool schema must keep up with the grammar — every