Skip to content

Commit 61802dc

Browse files
Trecekclaude
andauthored
Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content (#3995)
## Summary `open_kitchen` silently returns `content: ""` with `success: true` when post-prune validation detects dangling routes or route-consistency errors. The architectural weakness is threefold: 1. **`load_and_validate()` computes structural errors but discards them** — `LoadRecipeResult` has no `errors` field, so the reason for `valid=False` is lost at the TypedDict boundary. 2. **`open_kitchen` stamps `success: true` unconditionally** — it never checks `result["valid"]` or `result["content"]` before returning. 3. **`_prune_skipped_steps` produces unrepairable dangling routes silently** — when a pruned step has no computable redirect (no `on_success`, no `when=None` catch-all in `on_result`), the route-repair loop is a no-op, and the only signal is a list of strings returned from `_validate_no_dangling_routes` that gets appended to a local variable and then discarded. Part A addresses the core data-loss path: surfacing structural errors through `LoadRecipeResult`, making `open_kitchen` fail-closed on invalid content, and adding tests that catch this class of bug. Part B will address the recipe YAML fixes (adding `when=None` catch-all to `resolve_review`) and broader vacuous-assertion cleanup. ## Problem When the orchestrator backend is Codex, `open_kitchen(name="implementation")` returns `success: true`, `valid: false`, and `content: ""` — the **entire recipe body is silently withheld**. The orchestrator receives no explanation for the empty content, has no prompt instruction for this state, and is structurally blocked from every recovery path (`recipe_read_guard.py` PreToolUse hook + sous-chef raw-read prohibition + `load_recipe` returning the same empty result). In run `impl-20260610-095047-486925` (TalonT-Org/the-token-reserve#147, PR TalonT-Org/the-token-reserve#189, AutoSkillit 0.10.745, 2026-06-10 09:49–11:31), the Codex orchestrator **improvised the entire 60-minute pipeline from the Mermaid `diagram` field, the ingredients table, and the sous-chef prompt** — executing steps the composition had pruned as backend-incompatible (e.g. `implement` → `implement-worktree-no-merge`, declared `requires backend ['claude-code']`), with no step routing, no `capture:` contracts, no retry counts, and zero `record_pipeline_step` calls. The PR happened to merge green, but the run cannot be trusted to have followed the composed plan. **Reproduced at repo HEAD `18f410893` (0.10.748).** This is the sixth manifestation of the step-pruning/content-divergence family. ## Root cause chain 1. **Guard introduction (2026-06-09):** `207deda91` (#3960) introduced the `backend_supports_git_write` ingredient and added `skip_when_false: inputs.backend_supports_git_write` to 7 steps in `implementation.yaml` (and 7 in `remediation.yaml`) including `resolve_review` — without giving `resolve_review` a `when=None` catch-all route. The bug became reachable <24h before the failing run. 2. **Codex capability override:** `_backend_capability_overrides()` (`src/autoskillit/server/tools/_auto_overrides.py:19-28`) maps Codex's `git_metadata_writable=False` to `backend_supports_git_write="false"`, injected into session overrides at `src/autoskillit/server/tools/tools_kitchen.py:540`, promoted over user-supplied overrides via `_promote_capability_keys` (line 542), and passed to `load_and_validate` (lines 627-635). 3. **Unrepairable route:** `_prune_skipped_steps()` (`src/autoskillit/recipe/_recipe_composition.py:294-415` (redirect derivation 352-364)) prunes `resolve_review` but derives `redirect = None` — the step has no `on_success` and all four of its `on_result` conditions carry `when` clauses (the repair logic deliberately never derives a redirect from `on_failure`/`on_exhausted`) (`src/autoskillit/recipes/implementation.yaml:1089-1097`). The surviving `enrich_diff_context` (`on_success: resolve_review`, `on_failure: resolve_review`, `implementation.yaml:1064-1065`) is left dangling. The code comment acknowledges this path: "redirect stays None and `_validate_no_dangling_routes` catches dangling refs." 4. **Silent content wipe:** `_validate_no_dangling_routes` returns `["Step 'enrich_diff_context' routes to unknown step 'resolve_review'"]` and `load_and_validate()` sets `raw = ""` (`src/autoskillit/recipe/_api.py:415-418`). The `errors` list feeds only `compute_recipe_validity()`; **`LoadRecipeResult` has no `errors` field**, so the reason never reaches the caller. 5. **No fail signal, no recovery:** `open_kitchen` returns `success: true, kitchen: "open"` without checking `valid`/`content` (`tools_kitchen.py:690-692`; the deferred-recall branch at 605-607 is equally unchecked). Neither sous-chef SKILL.md nor any orchestrator prompt covers `content: ""`. The #3736/#3947 defenses (raw-read prohibition, `recipe_read_guard.py`, compaction disable) eliminate self-recovery, converting the state into a trap. ## Reproduction (repo HEAD `18f410893`, 0.10.748) ```python load_and_validate("implementation", project_dir=Path("/home/talon/projects/token-reserve"), backend_name="codex", lister=DefaultSkillResolver(), ingredient_overrides={"backend_supports_git_write": "false", "post_run_diagnostics": "false"}) # → valid: False, content_len: 0, 13 error-severity backend-incompatible-skill suggestions, # exactly one dangling-route error (instrumented): enrich_diff_context → resolve_review # same call with backend_name="claude-code" → valid: True, content_len: 86,942 ``` Claude Code is unaffected because `git_metadata_writable=True` produces no pruning of these steps — full content is delivered. This is **not** Codex-side truncation: the run's rollout log records the full `open_kitchen` results (90,635 and 86,672 bytes) with zero truncation markers; `tool_output_token_limit=50000` and compaction-disable were correctly written to `config.toml`. ## Remediation targets 1. **Surface the wipe reason:** add `errors: list[str]` to `LoadRecipeResult` and populate it whenever `raw` is cleared (`_api.py:415-426`). 2. **Fail loudly in `open_kitchen`:** when `valid is False` and `content == ""`, return a failure envelope (`success: false` + the dangling-route errors) — never `success: true` with no recipe. 3. **Orchestrator halt instruction:** sous-chef/orchestrator-prompt rule — if `open_kitchen` returns empty `content`, halt and escalate; never execute from the diagram. 4. **Fix the recipe:** give `resolve_review` a computable redirect (`when=None` catch-all) or guard `enrich_diff_context`'s routes consistently so the pruned subgraph is removed atomically; audit the other `backend_supports_git_write`-guarded steps — 11 in `implementation.yaml` at HEAD, added across `207deda91`, `884e1f2ae`, `d88bda83a` — and the other bundled recipes (`remediation.yaml` gained 7 guards in the same commit) for the same exposure. 5. **Close the test escape hatch:** replace the `continue` at `tests/recipe/test_hidden_ingredients.py:966-975` with an assertion (and ideally a semantic validation rule) that every prunable step has a computable redirect; add a codex-composition test asserting `valid is True` AND non-empty `content` AND all surviving steps present for each bundled recipe. 6. **Secondary:** bundled skills are invisible to Codex sessions (no `--plugin-dir` equivalent; `src/autoskillit/workspace/session_skills.py:319` excludes `BUNDLED`), so the orchestrator could not map a post-hoc "run analyze health" request to `/autoskillit:analyze-pipeline-health`; consider listing invocable bundled skill commands in the Codex orchestrator prompt or `open_kitchen` result. Closes #3992 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260610-175022-808728/.autoskillit/temp/rectify/rectify_silent_recipe_content_wipe_part_a_2026-06-10_180512.md` 🤖 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 | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | rectify* | opus[1m] | 1 | 4.9k | 23.2k | 1.3M | 97.0k | 43 | 82.9k | 21m 11s | | review_approach* | opus[1m] | 1 | 4.5k | 6.1k | 260.0k | 49.0k | 14 | 35.4k | 5m 23s | | dry_walkthrough* | opus | 1 | 41 | 9.6k | 867.8k | 89.3k | 35 | 67.7k | 7m 33s | | implement* | MiniMax-M3 | 1 | 3.7M | 9.0k | 0 | 0 | 79 | 0 | 7m 44s | | audit_impl* | opus[1m] | 1 | 1.2k | 11.3k | 257.3k | 48.2k | 18 | 39.9k | 4m 33s | | prepare_pr* | MiniMax-M3 | 1 | 284.5k | 2.9k | 0 | 0 | 12 | 0 | 1m 7s | | compose_pr* | MiniMax-M3 | 1 | 351.0k | 1.3k | 0 | 0 | 12 | 0 | 51s | | review_pr* | opus[1m] | 1 | 43 | 26.5k | 980.9k | 89.0k | 36 | 68.0k | 6m 17s | | **Total** | | | 4.4M | 89.8k | 3.6M | 97.0k | | 294.0k | 54m 42s | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | rectify | 0 | — | — | — | | review_approach | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 243 | 0.0 | 0.0 | 37.0 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | review_pr | 0 | — | — | — | | **Total** | **243** | 14903.1 | 1210.0 | 369.7 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 4 | 10.7k | 67.0k | 2.8M | 226.3k | 37m 26s | | opus | 1 | 41 | 9.6k | 867.8k | 67.7k | 7m 33s | | MiniMax-M3 | 3 | 4.4M | 13.2k | 0 | 0 | 9m 42s | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8c2ced0 commit 61802dc

30 files changed

Lines changed: 254 additions & 50 deletions

src/autoskillit/fleet/_api.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,14 +372,17 @@ async def _run_dispatch(
372372
)
373373

374374
if not validation_result.get("valid", False):
375+
structural_errors = validation_result.get("errors", [])
375376
error_findings = [
376377
s for s in validation_result.get("suggestions", []) if s.get("severity") == "error"
377378
]
379+
error_parts = structural_errors[:3] + [
380+
f"[{f['rule']}] {f['message']}" for f in error_findings[:3]
381+
]
378382
return DispatchResult(
379383
DispatchRejected(
380384
error_code=FleetErrorCode.FLEET_RECIPE_INVALID,
381-
message=f"Recipe '{recipe}' has validation errors: "
382-
+ "; ".join(f"[{f['rule']}] {f['message']}" for f in error_findings[:3]),
385+
message=f"Recipe '{recipe}' has validation errors: " + "; ".join(error_parts),
383386
),
384387
per_dispatch_state_path=None,
385388
)

src/autoskillit/hooks/formatters/_fmt_recipe.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
_FMT_LOAD_RECIPE_SUPPRESSED: frozenset[str] = frozenset(
3434
{
3535
"greeting", # delivered via positional CLI arg, not MCP response
36+
"errors", # structural validation errors; internal to load_and_validate
3637
"diagram", # user sees it in terminal preview; agent doesn't need it
3738
"kitchen_rules", # already in the YAML content
3839
"requires_packs", # internal field; used for skill gating, not display
@@ -170,6 +171,7 @@ def _fmt_load_recipe(data: LoadRecipeResult, pipeline: bool) -> str:
170171
{
171172
"success", # metadata — model infers success from formatted output
172173
"kitchen", # metadata — model knows kitchen state from context
174+
"errors", # structural validation errors; internal to load_and_validate
173175
"greeting", # delivered via CLI preview, not MCP response
174176
"diagram", # rendered in terminal preview, not needed by agent
175177
"kitchen_rules", # already embedded in YAML content

src/autoskillit/recipe/_api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ def load_and_validate(
312312
raw = substitute_scripts_placeholder(raw)
313313
suggestions: list[dict[str, Any]] = []
314314
valid = True
315+
errors: list[str] = []
315316
recipe = None
316317
active_recipe = None
317318
_skip_resolutions: dict[str, bool | None] = {}
@@ -517,6 +518,7 @@ def load_and_validate(
517518
_assert_no_raw_placeholders(raw, context=name, hidden_ingredient_names=_hidden_names)
518519
result: LoadRecipeResult = {
519520
"content": raw,
521+
"errors": errors,
520522
"diagram": diagram,
521523
"suggestions": suggestions,
522524
"valid": valid,

src/autoskillit/recipe/_recipe_ingredients.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ class LoadRecipeResult(TypedDict, total=False):
111111
"""Typed schema for the load_recipe handler → formatter boundary."""
112112

113113
content: str
114+
errors: list[str]
114115
diagram: str | None
115116
suggestions: list[dict[str, Any]]
116117
valid: bool
@@ -133,8 +134,9 @@ class OpenKitchenResult(TypedDict, total=False):
133134
Extends LoadRecipeResult with three post-return keys injected by the handler.
134135
"""
135136

136-
# Inherited from LoadRecipeResult (14 keys)
137+
# Inherited from LoadRecipeResult (15 keys)
137138
content: str
139+
errors: list[str]
138140
diagram: str | None
139141
suggestions: list[dict[str, Any]]
140142
valid: bool

src/autoskillit/recipes/implementation-groups.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,6 @@
10651065
"route": "check_review_loop"
10661066
},
10671067
{
1068-
"when": "true",
10691068
"route": "check_review_loop"
10701069
}
10711070
],
@@ -1110,6 +1109,9 @@
11101109
{
11111110
"when": "${{ result.verdict }} == 'ci_only_failure'",
11121111
"route": "release_issue_failure"
1112+
},
1113+
{
1114+
"route": "release_issue_failure"
11131115
}
11141116
],
11151117
"on_failure": "release_issue_failure",

src/autoskillit/recipes/implementation-groups.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -945,8 +945,7 @@ steps:
945945
route: check_review_loop
946946
- when: "${{ result.verdict }} == approved"
947947
route: check_review_loop
948-
- when: "true"
949-
route: check_review_loop
948+
- route: check_review_loop
950949
on_failure: check_repo_ci_event
951950
on_context_limit: check_repo_ci_event
952951
optional: true
@@ -986,6 +985,7 @@ steps:
986985
route: pre_review_rebase
987986
- when: "${{ result.verdict }} == 'ci_only_failure'"
988987
route: release_issue_failure
988+
- route: release_issue_failure
989989
on_failure: release_issue_failure
990990
optional: true
991991
skip_when_false: "inputs.backend_supports_git_write"

src/autoskillit/recipes/implementation.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1147,7 +1147,6 @@
11471147
"route": "check_review_loop"
11481148
},
11491149
{
1150-
"when": "true",
11511150
"route": "check_review_loop"
11521151
}
11531152
],
@@ -1209,6 +1208,9 @@
12091208
{
12101209
"when": "${{ result.verdict }} == 'ci_only_failure'",
12111210
"route": "release_issue_failure"
1211+
},
1212+
{
1213+
"route": "release_issue_failure"
12121214
}
12131215
],
12141216
"on_failure": "release_issue_failure",

src/autoskillit/recipes/implementation.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,8 +1035,7 @@ steps:
10351035
route: check_review_loop
10361036
- when: ${{ result.verdict }} == approved
10371037
route: check_review_loop
1038-
- when: 'true'
1039-
route: check_review_loop
1038+
- route: check_review_loop
10401039
on_failure: check_repo_ci_event
10411040
on_context_limit: check_repo_ci_event
10421041
optional: true
@@ -1095,6 +1094,7 @@ steps:
10951094
route: pre_review_rebase
10961095
- when: ${{ result.verdict }} == 'ci_only_failure'
10971096
route: release_issue_failure
1097+
- route: release_issue_failure
10981098
on_failure: release_issue_failure
10991099
optional: true
11001100
skip_when_false: inputs.backend_supports_git_write

src/autoskillit/recipes/merge-prs.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,9 @@
11961196
{
11971197
"when": "${{ result.verdict }} == 'no_test_infrastructure'",
11981198
"route": "register_clone_failure"
1199+
},
1200+
{
1201+
"route": "register_clone_failure"
11991202
}
12001203
],
12011204
"on_failure": "register_clone_failure",
@@ -1559,6 +1562,9 @@
15591562
{
15601563
"when": "${{ result.verdict }} == 'ci_only_failure'",
15611564
"route": "register_clone_failure"
1565+
},
1566+
{
1567+
"route": "register_clone_failure"
15621568
}
15631569
],
15641570
"on_failure": "register_clone_failure",

src/autoskillit/recipes/merge-prs.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,7 @@ steps:
11621162
route: register_clone_failure
11631163
- when: ${{ result.verdict }} == 'no_test_infrastructure'
11641164
route: register_clone_failure
1165+
- route: register_clone_failure
11651166
on_failure: register_clone_failure
11661167
skip_when_false: inputs.backend_supports_git_write
11671168
retries: 3
@@ -1523,6 +1524,7 @@ steps:
15231524
route: register_clone_failure
15241525
- when: ${{ result.verdict }} == 'ci_only_failure'
15251526
route: register_clone_failure
1527+
- route: register_clone_failure
15261528
on_failure: register_clone_failure
15271529
skip_when_false: inputs.backend_supports_git_write
15281530
note: 'Resolves review findings on the integration PR (REQ-REVIEW-002). Fetches

0 commit comments

Comments
 (0)