Rectify: Recipe Validation Envelope Hides Semantic Errors Behind "unknown structural error" (#4065)

## Summary

`_recipe_validation_error_response` in
`server/tools/tools_kitchen.py:100-115` constructs its
`user_visible_message` and `error` fields exclusively from
`result["errors"]` (the structural error list). When `valid=False` is
driven by ERROR-severity semantic or contract findings — which live in
`result["suggestions"]`, never in `result["errors"]` — the envelope
falls back to the literal string `"unknown structural error"`, hiding
every real error message from the user.

The architectural weakness is a **split-channel error model with no
consumer-side merge**: `compute_recipe_validity` (registry.py:245-254)
derives `valid` from three independent sources (`errors`, semantic
findings, contract findings), but the envelope builder reads only one
source. The fleet dispatch path (`fleet/_api.py:374-388`) already
handles this correctly by merging both channels — the open_kitchen
envelope is the sole consumer that doesn't.

## Implementation Plan

Plan file:
`/home/talon/projects/autoskillit-runs/remediation-20260611-063512-882312/.autoskillit/temp/rectify/rectify_envelope_hides_semantic_errors_2026-06-11_064500.md`

Closes #4060

🤖 Generated with [Claude Code](https://claude.com/claude-code) via
AutoSkillit
<!-- autoskillit:pipeline-signature
steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr
-->

## Token Usage Summary

| Step | Model | count | uncached | output | cache_read | peak_ctx |
turns | cache_write | time |

|------|-------|-------|----------|--------|------------|----------|-------|-------------|------|
| run_bem* | sonnet | 1 | 45.1k | 48.1k | 677.2k | 130.5k | 35 | 112.5k
| 17m 18s |
| dispatch:bf3ba4d0-338f-46e2-b744-1cc5af0cb74f* | sonnet | 1 | 66 |
6.1k | 324.1k | 48.4k | 17 | 29.4k | 18m 48s |
| plan* | opus[1m] | 1 | 78 | 23.7k | 1.8M | 103.4k | 46 | 104.1k | 13m
39s |
| verify* | sonnet | 1 | 799 | 10.1k | 596.8k | 55.8k | 33 | 34.9k | 5m
15s |
| implement* | MiniMax-M3 | 1 | 1.4M | 12.1k | 532.7k | 64.8k | 86 | 0 |
9m 43s |
| audit_impl* | sonnet | 1 | 190 | 9.1k | 248.7k | 45.8k | 18 | 34.9k |
4m 50s |
| prepare_pr* | MiniMax-M3 | 1 | 110.2k | 2.7k | 85.0k | 44.1k | 13 | 0
| 1m 39s |
| compose_pr* | MiniMax-M3 | 1 | 177.3k | 1.7k | 0 | 0 | 11 | 0 | 1m 5s
|
| review_pr* | sonnet | 1 | 156 | 24.6k | 948.4k | 75.1k | 44 | 98.7k |
7m 31s |
| resolve_review* | sonnet | 1 | 133 | 9.9k | 833.6k | 66.4k | 37 |
46.2k | 5m 37s |
| **Total** | | | 1.8M | 147.9k | 6.0M | 130.5k | | 460.5k | 1h 25m |

\* *Step used a non-Anthropic provider; caching behavior may differ.*

## Token Efficiency

| Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC |
|------|-------------|----------------|-----------------|------------|
| run_bem | 0 | — | — | — |
| dispatch:bf3ba4d0-338f-46e2-b744-1cc5af0cb74f | 0 | — | — | — |
| plan | 0 | — | — | — |
| verify | 0 | — | — | — |
| implement | 68 | 7833.8 | 0.0 | 177.4 |
| audit_impl | 0 | — | — | — |
| prepare_pr | 0 | — | — | — |
| compose_pr | 0 | — | — | — |
| review_pr | 0 | — | — | — |
| resolve_review | 0 | — | — | — |
| **Total** | **68** | 88226.4 | 6772.4 | 2175.4 |

## Model Usage Breakdown

| Model | steps | uncached | output | cache_read | cache_write | time |
|-------|-------|----------|--------|------------|-------------|------|
| sonnet | 6 | 46.4k | 107.8k | 3.6M | 356.4k | 59m 21s |
| opus[1m] | 1 | 78 | 23.7k | 1.8M | 104.1k | 13m 39s |
| MiniMax-M3 | 3 | 1.7M | 16.4k | 617.7k | 0 | 12m 28s |

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Trecek

src/autoskillit/server/tools/tools_kitchen.py

@@ -99,14 +99,23 @@ def _kitchen_failure_envelope(
 
 def _recipe_validation_error_response(name: str, result: dict[str, Any]) -> str:
     _errs: list[str] = result.get("errors", [])
-    _error_detail = "; ".join(_errs[:3]) if _errs else "unknown structural error"
+    _error_findings = [
+        s
+        for s in result.get("suggestions", [])
+        if isinstance(s, dict) and s.get("severity") == "error"
+    ]
+    _finding_strs = [f"[{f.get('rule', '')}] {f.get('message', '')}" for f in _error_findings]
+    if _errs and _finding_strs:
+        _error_parts = _errs[:2] + _finding_strs[:1]
+    else:
+        _error_parts = (_errs + _finding_strs)[:3]
+    _error_detail = "; ".join(_error_parts) if _error_parts else "unknown structural error"
+    _label = "structural validation" if (_errs and not _error_findings) else "validation"
     return json.dumps(
         {
             "success": False,
             "kitchen": "failed",
-            "user_visible_message": (
-                f"Recipe '{name}' failed structural validation: {_error_detail}"
-            ),
+            "user_visible_message": (f"Recipe '{name}' failed {_label}: {_error_detail}"),
             "error": f"Recipe '{name}' failed validation: {_error_detail}",
             "stage": "recipe_validation",
             "errors": _errs,

tests/contracts/AGENTS.md

@@ -39,6 +39,7 @@ Protocol satisfaction, package gateway, and skill contract compliance tests.
 | `test_github_ops.py` | Contract tests: GitHub operation semantics in SKILL.md files |
 | `test_hook_bridge_coverage.py` | REQ-BRIDGE-001: quota guard hook config bridge must produce exactly the keys that resolve_quota_settings() reads |
 | `test_implement_experiment_contracts.py` | Contract tests for implement-experiment SKILL.md — test infrastructure requirements |
+| `test_validation_error_envelope_coverage.py` | REQ-ENVELOPE-001: validation error envelope must surface all error channels from compute_recipe_validity |
 | `test_input_type_semantic_correctness.py` | Cross-validate skill_contracts.yaml path input types against SKILL.md content |
 | `test_instruction_surface.py` | Contract tests: every instruction surface must carry the pipeline tool restriction |
 | `test_issue_body_discipline.py` | Cross-skill contract: no SKILL.md may append validation summaries to issue bodies |

tests/contracts/test_validation_error_envelope_coverage.py

@@ -0,0 +1,35 @@
+"""REQ-ENVELOPE-001: _recipe_validation_error_response must surface all error channels
+from compute_recipe_validity (structural errors + semantic/contract findings).
+
+Mirrors the test_hook_bridge_coverage pattern: when a new error channel is added to
+LoadRecipeResult, this test forces the envelope to surface it.
+"""
+
+from __future__ import annotations
+
+import json
+
+import pytest
+
+pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small]
+
+
+def test_envelope_surfaces_both_structural_and_semantic_channels():
+    """Envelope must reference at least one item from errors AND from suggestions."""
+    from autoskillit.server.tools.tools_kitchen import _recipe_validation_error_response
+
+    result = {
+        "valid": False,
+        "errors": ["structural problem"],
+        "suggestions": [
+            {
+                "severity": "error",
+                "rule": "semantic-rule",
+                "message": "semantic problem",
+                "step": "s",
+            },
+        ],
+    }
+    response = json.loads(_recipe_validation_error_response("demo", result))
+    assert "structural problem" in response["user_visible_message"]
+    assert "semantic-rule" in response["user_visible_message"]

tests/infra/test_schema_version_convention.py

@@ -119,11 +119,11 @@ def _is_yaml_dump(node: ast.expr) -> bool:
     # _lifespan.py — hooks.json self-heal on startup drift (co-owned with Claude plugin system)
     ("src/autoskillit/server/_lifespan.py", 87),
     # tools_kitchen.py — hook config, quota guard, git_ops_policy, ingredient locks overlay
-    ("src/autoskillit/server/tools/tools_kitchen.py", 160),
-    ("src/autoskillit/server/tools/tools_kitchen.py", 179),
-    ("src/autoskillit/server/tools/tools_kitchen.py", 213),
-    ("src/autoskillit/server/tools/tools_kitchen.py", 863),
-    ("src/autoskillit/server/tools/tools_kitchen.py", 923),
+    ("src/autoskillit/server/tools/tools_kitchen.py", 169),
+    ("src/autoskillit/server/tools/tools_kitchen.py", 188),
+    ("src/autoskillit/server/tools/tools_kitchen.py", 222),
+    ("src/autoskillit/server/tools/tools_kitchen.py", 872),
+    ("src/autoskillit/server/tools/tools_kitchen.py", 932),
     # tools_pipeline_tracker.py — tracker_data dict
     ("src/autoskillit/server/tools/tools_pipeline_tracker.py", 166),
     # tools_status.py — mcp_data dict

tests/server/test_tools_kitchen_envelope.py

@@ -883,3 +883,134 @@ def test_every_return_path_parses_as_json_and_has_boolean_success(stage):
 
     envelope = json.loads(_kitchen_failure_envelope(RuntimeError("test"), stage=stage))
     assert isinstance(envelope["success"], bool)
+
+
+def test_recipe_validation_error_response_surfaces_semantic_errors():
+    """_recipe_validation_error_response must merge error-severity suggestions
+    into the user-visible message when errors is empty."""
+    from autoskillit.server.tools.tools_kitchen import _recipe_validation_error_response
+
+    result = {
+        "valid": False,
+        "errors": [],
+        "suggestions": [
+            {
+                "severity": "error",
+                "rule": "backend-incompatible-skill",
+                "message": "Step 'deploy' uses skill 'merge_worktree'",
+                "step": "deploy",
+            },
+        ],
+    }
+    response = json.loads(_recipe_validation_error_response("demo", result))
+    assert response["success"] is False
+    assert response["kitchen"] == "failed"
+    assert response["stage"] == "recipe_validation"
+    assert "unknown structural error" not in response["user_visible_message"]
+    assert "backend-incompatible-skill" in response["user_visible_message"]
+    assert "merge_worktree" in response["error"]
+
+
+@pytest.mark.parametrize(
+    "result,expected_substring",
+    [
+        (
+            {"valid": False, "errors": ["schema violation"], "suggestions": []},
+            "schema violation",
+        ),
+        (
+            {
+                "valid": False,
+                "errors": [],
+                "suggestions": [
+                    {
+                        "severity": "error",
+                        "rule": "rule-x",
+                        "message": "bad step",
+                        "step": "s",
+                    }
+                ],
+            },
+            "rule-x",
+        ),
+        (
+            {
+                "valid": False,
+                "errors": [],
+                "suggestions": [
+                    {
+                        "severity": "error",
+                        "rule": "contract-y",
+                        "message": "stale",
+                        "step": "c",
+                    }
+                ],
+            },
+            "contract-y",
+        ),
+    ],
+)
+def test_validation_error_envelope_always_names_cause(result, expected_substring):
+    """Envelope must always name a cause when valid=False, never 'unknown structural error'."""
+    from autoskillit.server.tools.tools_kitchen import _recipe_validation_error_response
+
+    response = json.loads(_recipe_validation_error_response("demo", result))
+    assert response["success"] is False
+    assert response["kitchen"] == "failed"
+    assert response["stage"] == "recipe_validation"
+    assert "unknown structural error" not in response["user_visible_message"]
+    assert expected_substring in response["user_visible_message"]
+
+
+@pytest.mark.anyio
+async def test_open_kitchen_fails_on_semantic_errors_only(tmp_path, monkeypatch):
+    """open_kitchen must surface semantic error findings when errors=[]."""
+    monkeypatch.chdir(tmp_path)
+    mock_ctx = _make_mock_ctx()
+    mock_ctx.enable_components = AsyncMock()
+    mock_ctx.recipes = MagicMock()
+    mock_ctx.recipes.load_and_validate.return_value = {
+        "content": "name: demo\nsteps:\n  do:\n    tool: run_cmd\n",
+        "valid": False,
+        "errors": [],
+        "suggestions": [
+            {
+                "severity": "error",
+                "rule": "backend-incompatible-skill",
+                "message": "Step 'deploy' uses skill 'merge_worktree'",
+                "step": "deploy",
+            },
+        ],
+    }
+    mock_ctx.config.migration.suppressed = []
+
+    with patch("autoskillit.server._get_ctx", return_value=mock_ctx):
+        with patch("autoskillit.server.logger"):
+            with patch(
+                "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock()
+            ):
+                with patch("autoskillit.server.tools.tools_kitchen._write_hook_config"):
+                    from autoskillit.server.tools.tools_kitchen import open_kitchen
+
+                    result_str = await open_kitchen(name="demo", ctx=mock_ctx)
+
+    parsed = json.loads(result_str)
+    assert parsed["success"] is False
+    assert "unknown structural error" not in parsed["user_visible_message"]
+    assert "backend-incompatible-skill" in parsed["user_visible_message"]
+
+
+def test_recipe_validation_error_response_handles_malformed_suggestions():
+    """_recipe_validation_error_response must not crash on suggestions missing rule/message."""
+    from autoskillit.server.tools.tools_kitchen import _recipe_validation_error_response
+
+    result = {
+        "valid": False,
+        "errors": [],
+        "suggestions": [
+            {"severity": "error", "step": "some-step"},
+        ],
+    }
+    response = json.loads(_recipe_validation_error_response("demo", result))
+    assert "unknown structural error" not in response["user_visible_message"]
+    assert response["success"] is False