Skip to content

Recipe validation envelope hides semantic errors behind 'unknown structural error' #4060

Description

@Trecek

Summary

When recipe validation fails with valid=False but no structural errors, the open_kitchen failure envelope reports the meaningless detail "unknown structural error" — the actual cause (ERROR-severity semantic/contract findings) sits unrendered inside suggestions. This is exactly what made the #4059 regression undiagnosable from the user-facing message:

Recipe 'implementation' failed structural validation: unknown structural error (errors=[], while 13 ERROR-severity findings sat in suggestions)

Root cause

_recipe_validation_error_response (v0.10.752 server/tools/tools_kitchen.py:100-115, added by PR #3995) composes its detail exclusively from the structural list:

_errs: list[str] = result.get("errors", [])
_error_detail = "; ".join(_errs[:3]) if _errs else "unknown structural error"

But compute_recipe_validity (recipe/registry.py:245-254) returns False for any of THREE sources: structural errors, ERROR-severity semantic findings, or error-severity contract findings. Sources 2 and 3 live in result["suggestions"], never in result["errors"]. Whenever valid=False is driven by semantic/contract findings only, the envelope renders the fallback string and misdiagnoses a semantic failure as structural.

By contrast, the fleet rejection path (fleet/_api.py:374-388) already merges error-severity suggestions into its message — the open_kitchen envelope should not be weaker.

Scope of fix

  1. _recipe_validation_error_response must merge error-severity items from result.get("suggestions", []) into the rendered detail (and user_visible_message) whenever the structural errors list is empty — making "unknown structural error" structurally unreachable, since valid=False always has a nameable cause.

  2. Add a producer/consumer contract test for the LoadRecipeResulttools_kitchen.py boundary, following the existing pattern in tests/contracts/test_hook_bridge_coverage.py: every key the consumer gate-checks (valid, content, errors, suggestions) must be populated by the real load_and_validate at runtime (the existing test_load_recipe_result_is_typed only checks TypedDict annotations, not runtime values).

  3. Encode the envelope invariant in a test: construct a result with valid=False, errors=[], and one error-severity suggestion; assert the envelope message names the finding's rule/message rather than "unknown structural error". (This is the exact shape production produced in Pre-prune validity computation breaks Codex open_kitchen and fleet dispatch #4059 and the one shape no Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 test mocked.)

  4. Render merged findings with the fleet format [{rule}] {message} (matching fleet/_api.py:380) so both failure paths phrase identical conditions identically. Deliberate-duplication note: the near-identical merging logic at fleet/_api.py:374-380 is NOT unified here, to keep zero file overlap with Pre-prune validity computation breaks Codex open_kitchen and fleet dispatch #4059 (which modifies fleet/_api.py); extract a shared helper in a follow-up after both merge (AGENTS.md "Avoid Redundancy" — this duplication is temporary and intentional).

Files (parallel-safe vs #4059 — zero overlap)

  • src/autoskillit/server/tools/tools_kitchen.py (_recipe_validation_error_response only)
  • tests/server/test_tools_kitchen_envelope.py
  • tests/contracts/ (new boundary contract test)

Note: open T5 WP #4036 wires a generic _validate_result shape helper into other return paths of the same file (happy-path exits), and #4002 adds deferred-recall guard coverage in tests/server/test_open_kitchen_deferred_recall.py. Different functions/tests — minor, easily-resolved conflicts at worst.

Adjacent finding (out of scope, surfaced during adversarial validation)

A recipe YAML that parses to a dict WITHOUT a steps key bypasses every validation stage: valid stays True (initialized at _api.py:314) and the raw content is served as-is — the no-steps else branch (_api.py:458-459) only records timing. A silent-pass correctness gap, not an envelope-rendering problem; this ticket neither fixes nor worsens it. Now fully investigated and tracked as #4063.

Relationship to #4059

Independent and complementary: #4059 fixes WHY valid=False was wrong; this ticket fixes the envelope so that any future legitimate valid=False is diagnosable from the message. Either can merge first.

Investigation

Root cause fully established by the deep investigation attached to #4059 (see its "Investigation" section — Root Cause step 6, Similar Patterns, and Recommendation #2). No further investigation required; this ticket is implementation of validated recommendation #2.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions