fix: make format warnings reach execution boundary and fix schema handling defects

This commit addresses multiple critical schema and validation issues:
1. Adds `warn_format_violations` and ensures it runs at input/output validation boundaries, so format warnings now fire for actual module invocations instead of only direct schema validation calls
2. Fixes format warning walk to reach into combinators (anyOf/oneOf/allOf/additionalProperties) and avoid duplicate reports
3. Reworks type array handling to properly create real unions instead of collapsing to first member
4. Makes `type` and combinator siblings (enum/const/anyOf/etc) both enforced instead of discarding one
5. Honours object form of `additionalProperties` instead of ignoring it
6. Enforces `not` keyword instead of aborting schema generation
7. Fixes option keyword leakage across types and restores missing description/title fields on generated models
8. Adds comprehensive test coverage for all these changes

Signed-off-by: tercel <tercel.yi@gmail.com>

Author: tercel

CHANGELOG.md

@@ -6,6 +6,31 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 
+## [Unreleased]
+
+> **Release note:** this section contains BREAKING changes. It must ship as a
+> **minor** (or major) version bump, never a patch.
+
+### Changed
+
+- **BREAKING: a `type` array converts to a real union instead of collapsing to its first member.** `_schema_to_field_info` kept only the first non-`null` member of `{"type": [...]}` and dropped the rest, which failed in both directions. When the leading member was a scalar the annotation was **over-tightened**: `{"type": ["string", "boolean"]}` — what apexe emits for a value-optional flag — became `str`, so `color=true` was rejected while apcore-rust accepted it. When the leading member was `object` or `array` the annotation **widened to `Any`**, because `_TYPE_MAP` has no entry for either, so `{"type": ["object", "null"]}` accepted `42` and `"str"` alike. Each member now becomes its own union branch, and the type-specific option keywords are kept per branch, so `{"type": ["string", "integer"], "minLength": 3, "minimum": 10}` no longer applies the numeric bound to the string branch. **Impact:** a value that was accepted only because the union collapsed to `Any` now fails with `SCHEMA_VALIDATION_ERROR`; a value that was rejected only because a branch was discarded now succeeds. **Migration:** none for a well-formed call — this is the behaviour apcore-rust has always had.
+
+- **BREAKING: a combinator sibling of a `type` keyword is enforced, and `type` is no longer discarded by one.** The dispatch chain returned on the first keyword it matched, so `type` and its siblings could never both hold. `const`/`enum`/`oneOf`/`anyOf`/`allOf` were checked *before* the type dispatch and won outright: `{"type": "string", "anyOf": [{"minLength": 3}]}` annotated the field `Any` and accepted `12345`, `{"a": 1}`, and `True`. `type` and a combinator keyword are independent assertions that must both hold (JSON Schema 2020-12 §10.2), so the type-derived annotation is now intersected with a jsonschema-backed check for every sibling it does not already cover — the same engine `hardening.validate_schema_dict` uses, which keeps the two validation paths in agreement. **Impact:** a value that passed only because one of the two assertions was dropped now fails with `SCHEMA_VALIDATION_ERROR`. **Migration:** none for a well-formed call.
+
+- **BREAKING: the object form of `additionalProperties` is honoured.** `generate_model` only inspected `additionalProperties is False`; a sub-schema form (`{"type": "integer"}`) fell through to Pydantic's default `extra="ignore"`, so an undeclared key of any type was silently accepted and dropped. It now maps to `extra="allow"` plus a typed `__pydantic_extra__`, so undeclared keys are kept and their values validated. `additionalProperties: false` is unchanged. **Impact:** a call passing an undeclared key whose value does not match the declared sub-schema now fails with `SCHEMA_VALIDATION_ERROR`; undeclared keys that do match are now **retained** rather than dropped. **Migration:** none for a well-formed call — apcore-rust (jsonschema crate) has always rejected these.
+
+- **BREAKING: `not` is enforced instead of aborting schema generation.** `'not' keyword not yet supported` was raised at *model-build* time, so a module whose contract carried `not` anywhere could not be registered at all. It is now applied as a sibling assertion like every other combinator. **Impact:** a contract that previously failed to load now loads and validates; a value the `not` excludes is now rejected at the validation boundary. `if`/`then`/`else` still raise — unchanged and out of scope here.
+
+### Fixed
+
+- **Option keywords no longer leak across types, and no longer vanish on a required field.** Two related defects in `_build_field`. First, every constraint was applied field-wide regardless of the declared type, so `{"type": ["string", "integer"], "minimum": 10}` attached `ge=10` to the string branch. Constraints for a `type` array now live on their own branch. Second, the `enum`/`const`/combinator branches returned a bare `Field(default=...)`, discarding every constraint — but only for **required** fields, because the optional path rebuilt the field through `_clone_field_with_default` and picked them back up. `{"type": "string", "minLength": 5, "enum": ["ab", "abcdef"]}` therefore accepted `"ab"` when required and rejected it when optional. Both paths now build the field the same way.
+
+- **`description` and `title` reach the generated model.** `_build_field` never copied either keyword, so both were dropped for every property — a scalar `type` as much as a `type` array. Pydantic then re-derived a title from the field name, which made the loss easy to miss. LLM-facing exports (MCP / OpenAI / Anthropic tool definitions) and `content_hash` read the raw JSON Schema rather than the generated model, so they were unaffected; only direct consumers of `generate_model()` saw the omission.
+
+- **The SHOULD-level format warning reaches the execution boundary.** `_check_formats_and_warn` was only ever called from `hardening.validate_schema_dict`, which `SchemaValidator.validate` invokes solely when the schema carries a **top-level** `oneOf`/`anyOf`. Module invocation validates through `input_schema.model_validate` (`builtin_steps.py`), which never reached it, so a format violation on a real call emitted nothing at all — the conformance fixture `schema_hardening_formats.json`'s `warn_logged: true` half was satisfied only by tests calling `validate_schema_dict` directly. The new `hardening.warn_format_violations(data, model)` is now called from both the input and output validation steps, reading the source JSON Schema that `SchemaLoader` attaches to every generated model. Whether a schema declares any `format` at all is computed once and cached on the model, so a schema without one costs a single attribute lookup. A natively declared Pydantic model (no source schema) is skipped.
+
+- **The format warning walk reaches into combinators.** The walk descended only through `properties` and `items`, so a `format` inside an `anyOf` / `oneOf` / `allOf` branch or an `additionalProperties` sub-schema never warned. Each node is now checked against its own `format` before the walk descends, and the walk covers those four node kinds. A union branch is only descended into when the data actually satisfies it, so a sibling branch cannot report a format the value never carried, and an annotation reached through more than one branch is reported once. An unrecognised format still never warns and never fails, per JSON Schema 2020-12 §7.2.1.
+
 ## [0.26.0] - 2026-07-13
 
 ### Added

src/apcore/builtin_steps.py

@@ -41,6 +41,7 @@
     StepResult,
 )
 from apcore.policy import ExecutionPolicy, PolicyDecision
+from apcore.schema.hardening import warn_format_violations
 from apcore.config import Config
 from apcore.utils.call_chain import guard_call_chain
 
@@ -757,6 +758,8 @@ async def execute(self, ctx: PipelineContext) -> StepResult:
                 errors=errors,
             ) from exc
 
+        warn_format_violations(ctx.inputs, input_schema)
+
         ctx.validated_inputs = ctx.inputs
 
         # Redact sensitive fields after successful validation
@@ -907,6 +910,8 @@ async def execute(self, ctx: PipelineContext) -> StepResult:
                 errors=errors,
             ) from exc
 
+        warn_format_violations(ctx.output, output_schema)
+
         ctx.validated_output = ctx.output
 
         # Store redacted output as first-class Context field (symmetric with

src/apcore/schema/hardening.py

@@ -25,7 +25,7 @@
 
 from apcore.schema.types import SchemaValidationErrorDetail, SchemaValidationResult
 
-__all__ = ["content_hash", "validate_schema_dict"]
+__all__ = ["content_hash", "validate_schema_dict", "warn_format_violations"]
 
 logger = logging.getLogger(__name__)
 
@@ -130,6 +130,46 @@ def validate_schema_dict(data: Any, schema: dict[str, Any]) -> SchemaValidationR
     return SchemaValidationResult(valid=True, errors=[])
 
 
+def warn_format_violations(data: Any, model: Any) -> None:
+    """Emit the SHOULD-level format warnings for *data* against *model*'s source schema.
+
+    Module invocation validates through Pydantic, which has no format-annotation
+    concept, so without this call the warnings would only ever fire on the
+    `validate_schema_dict` path — which the executor does not reach. Models carry
+    their source JSON Schema as `__apcore_source_schema__` (set by `SchemaLoader`);
+    a model without one (a natively declared Pydantic model) is skipped.
+
+    Whether the schema declares any `format` at all is computed once and cached on
+    the model, so the common no-format schema costs a single attribute lookup.
+    """
+    source_schema = getattr(model, "__apcore_source_schema__", None)
+    if not isinstance(source_schema, dict):
+        return
+
+    declares_format = getattr(model, "__apcore_declares_format__", None)
+    if declares_format is None:
+        declares_format = _declares_format(source_schema)
+        try:
+            model.__apcore_declares_format__ = declares_format
+        except (AttributeError, TypeError):  # pragma: no cover - defensive
+            pass
+    if not declares_format:
+        return
+
+    _check_formats_and_warn(data, source_schema)
+
+
+def _declares_format(node: Any) -> bool:
+    """Return True when *node* carries a `format` keyword anywhere in its tree."""
+    if isinstance(node, dict):
+        if "format" in node:
+            return True
+        return any(_declares_format(value) for value in node.values())
+    if isinstance(node, list):
+        return any(_declares_format(item) for item in node)
+    return False
+
+
 def _map_error_code(errors: list[JsonschemaError]) -> str:
     """Map jsonschema validation errors to apcore error codes."""
     for error in errors:
@@ -156,7 +196,7 @@ def _error_to_detail(error: JsonschemaError) -> SchemaValidationErrorDetail:
     )
 
 
-def _check_formats_and_warn(data: Any, schema: Any, _path: str = "") -> None:
+def _check_formats_and_warn(data: Any, schema: Any, _path: str = "", _seen: set[Any] | None = None) -> None:
     """Walk the data/schema tree and log warnings for format violations.
 
     Sync finding A-D-032: previously this function only iterated
@@ -165,11 +205,21 @@ def _check_formats_and_warn(data: Any, schema: Any, _path: str = "") -> None:
     in Python while apcore-typescript and apcore-rust did warn. The
     walker now recurses into nested ``properties`` AND ``items`` so
     cross-language conformance fixtures see the same warning set.
+
+    Each node is checked against its own ``format`` before the walk descends,
+    which also covers combinator nodes: ``anyOf`` / ``oneOf`` branches (only the
+    ones the data actually satisfies, so a sibling branch cannot report a format
+    the value never carried) and every ``allOf`` member. An annotation reached
+    through more than one branch is reported once.
     """
     if not isinstance(schema, dict):
         return
+    if _seen is None:
+        _seen = set()
+
+    _warn_if_format_violated(data, schema, _path, _seen)
 
-    # Object: walk each property + recurse on its sub-schema
+    # Object: recurse on each declared property's sub-schema.
     if isinstance(data, dict):
         properties = schema.get("properties", {})
         if isinstance(properties, dict):
@@ -180,34 +230,59 @@ def _check_formats_and_warn(data: Any, schema: Any, _path: str = "") -> None:
                 if value is None:
                     continue
                 child_path = f"{_path}/{prop_name}" if _path else f"/{prop_name}"
-                fmt = prop_schema.get("format")
-                if fmt and isinstance(value, str):
-                    checker = _FORMAT_CHECKERS.get(fmt)
-                    if checker is not None and not checker(value):
-                        logger.warning(
-                            "Format violation (non-fatal): field %r declared format=%r but value %r is not conformant",
-                            child_path,
-                            fmt,
-                            value,
-                        )
-                # Recurse into nested objects and arrays.
-                _check_formats_and_warn(value, prop_schema, child_path)
+                _check_formats_and_warn(value, prop_schema, child_path, _seen)
+
+        additional = schema.get("additionalProperties")
+        if isinstance(additional, dict):
+            declared = properties if isinstance(properties, dict) else {}
+            for key, value in data.items():
+                if key in declared or value is None:
+                    continue
+                child_path = f"{_path}/{key}" if _path else f"/{key}"
+                _check_formats_and_warn(value, additional, child_path, _seen)
 
     # Array: walk each element against the schema's `items` declaration.
     if isinstance(data, list):
         items_schema = schema.get("items")
         if isinstance(items_schema, dict):
             for idx, value in enumerate(data):
-                child_path = f"{_path}[{idx}]"
-                if isinstance(value, str):
-                    fmt = items_schema.get("format")
-                    if fmt:
-                        checker = _FORMAT_CHECKERS.get(fmt)
-                        if checker is not None and not checker(value):
-                            logger.warning(
-                                "Format violation (non-fatal): field %r declared format=%r but value %r is not conformant",
-                                child_path,
-                                fmt,
-                                value,
-                            )
-                _check_formats_and_warn(value, items_schema, child_path)
+                _check_formats_and_warn(value, items_schema, f"{_path}[{idx}]", _seen)
+
+    # Combinators: a union branch annotates the data only when the data satisfies it.
+    for keyword in ("anyOf", "oneOf"):
+        branches = schema.get(keyword)
+        if isinstance(branches, list):
+            for branch in branches:
+                if isinstance(branch, dict) and Draft202012Validator(branch).is_valid(data):
+                    _check_formats_and_warn(data, branch, _path, _seen)
+
+    members = schema.get("allOf")
+    if isinstance(members, list):
+        for member in members:
+            if isinstance(member, dict):
+                _check_formats_and_warn(data, member, _path, _seen)
+
+
+def _warn_if_format_violated(data: Any, schema: dict[str, Any], path: str, seen: set[Any]) -> None:
+    """Log the SHOULD-level warning when *data* does not satisfy this node's ``format``.
+
+    An unrecognised format is skipped: JSON Schema 2020-12 §7.2.1 puts ``format`` in
+    the format-annotation vocabulary, where a format the implementation does not
+    recognise is collected as an annotation, never treated as a failure.
+    """
+    fmt = schema.get("format")
+    if not fmt or not isinstance(data, str):
+        return
+    checker = _FORMAT_CHECKERS.get(fmt)
+    if checker is None or checker(data):
+        return
+    key = (path, fmt, data)
+    if key in seen:
+        return
+    seen.add(key)
+    logger.warning(
+        "Format violation (non-fatal): field %r declared format=%r but value %r is not conformant",
+        path,
+        fmt,
+        data,
+    )