Skip to content

Commit fbfb358

Browse files
Trecekclaude
andauthored
[FIX] Double open_kitchen Context Cost — Pre-Reveal / Deferred-Recall Architectural Immunity (#4092)
## Summary The `open_kitchen` MCP tool lacks a unified gate-infrastructure / content-delivery separation. When the Codex backend pre-reveals kitchen tools at lifespan boot (`_pre_reveal_kitchen`), it opens the gate and performs side effects (hook config, quota prime, registry) but does NOT set `ctx.recipe_name`. When the agent subsequently calls `open_kitchen(name="X")`, the `_is_deferred_recall` guard fails (`recipe_name == ""` != `"X"`), causing the full `_open_kitchen_handler()` to re-execute — redundantly re-opening the gate, generating a new `kitchen_id`, re-writing hook config, re-priming quota, and restarting the quota refresh loop. Then the recipe is loaded and the full ~100KB payload is returned. The pre-reveal work is wasted and the gate infrastructure is initialized twice. Additionally, the `_is_deferred_recall` path (lines 595-683 of `tools_kitchen.py`) returns the **full** `LoadRecipeResult` including the `content` field (~72-102KB recipe YAML), with no `ingredients_only` stripping applied. This means every deferred recall re-sends the entire recipe. The architectural weakness is that `open_kitchen` conflates three concerns into a single code path: (1) gate infrastructure setup, (2) recipe content loading, and (3) response payload assembly. The `_is_deferred_recall` flag is a boolean that either skips or runs the handler wholesale, with no intermediate state for "gate already open, just load recipe." Closes #4089 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260612-091855-490847/.autoskillit/temp/rectify/rectify_double_open_kitchen_context_cost_2026-06-12_091855.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 | 7.3k | 20.6k | 2.7M | 120.6k | 72 | 99.9k | 25m 37s | | review_approach* | opus[1m] | 1 | 24 | 6.4k | 222.3k | 52.6k | 12 | 35.0k | 4m 54s | | dry_walkthrough* | opus | 1 | 40 | 11.9k | 890.7k | 95.9k | 36 | 96.1k | 6m 50s | | implement* | MiniMax-M3 | 1 | 5.4M | 14.7k | 0 | 0 | 115 | 0 | 13m 16s | | audit_impl* | opus[1m] | 1 | 34 | 14.4k | 280.3k | 63.5k | 20 | 48.7k | 9m 2s | | prepare_pr* | MiniMax-M3 | 1 | 292.3k | 3.3k | 0 | 0 | 21 | 0 | 1m 7s | | compose_pr* | MiniMax-M3 | 1 | 348.4k | 1.3k | 0 | 0 | 14 | 0 | 58s | | **Total** | | | 6.0M | 72.6k | 4.1M | 120.6k | | 279.7k | 1h 1m | \* *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 | 280 | 0.0 | 0.0 | 52.5 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **280** | 14712.9 | 999.1 | 259.1 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 3 | 7.4k | 41.4k | 3.2M | 183.7k | 39m 33s | | opus | 1 | 40 | 11.9k | 890.7k | 96.1k | 6m 50s | | MiniMax-M3 | 3 | 6.0M | 19.3k | 0 | 0 | 15m 21s | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ecdc61f commit fbfb358

9 files changed

Lines changed: 248 additions & 9 deletions

File tree

src/autoskillit/pipeline/context.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ class ToolContext:
168168
recipe_content_hash: str = field(default="")
169169
recipe_composite_hash: str = field(default="")
170170
recipe_version: str = field(default="")
171+
gate_infrastructure_ready: bool = field(default=False)
171172
kitchen_id: str = field(default="")
172173
active_recipe_packs: frozenset[str] | None = field(default_factory=lambda: None)
173174
active_recipe_features: frozenset[str] | None = field(default_factory=lambda: None)

src/autoskillit/server/_lifespan.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ async def _pre_reveal_kitchen(ctx: Any) -> None:
342342
register_active_kitchen(ctx.kitchen_id, os.getpid(), str(ctx.project_dir))
343343
_write_hook_config()
344344
await _prime_quota_cache()
345+
ctx.gate_infrastructure_ready = True
345346

346347

347348
async def _food_truck_auto_gate_boot(ctx: Any) -> None:

src/autoskillit/server/tools/tools_kitchen.py

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ async def _open_kitchen_handler() -> str | None:
276276
except Exception:
277277
logger.warning("open_kitchen_registry_failed", exc_info=True)
278278

279+
ctx.gate_infrastructure_ready = True
279280
return None
280281

281282

@@ -331,6 +332,7 @@ def _close_kitchen_handler() -> None:
331332
ctx.recipe_content_hash = ""
332333
ctx.recipe_composite_hash = ""
333334
ctx.recipe_version = ""
335+
ctx.gate_infrastructure_ready = False
334336
logger.info("close_kitchen", gate_state="closed")
335337
if (log := ctx.github_api_log) is not None:
336338
orphan_usage = log.drain(ctx.kitchen_id)
@@ -515,15 +517,30 @@ async def open_kitchen(
515517
disabled_subsets = _get_ctx().config.subsets.disabled
516518

517519
_ctx_pre = _get_ctx()
518-
_is_deferred_recall = (
519-
name is not None and _ctx_pre.gate.enabled and _ctx_pre.recipe_name == name
520-
)
520+
_skip_handler = _ctx_pre.gate_infrastructure_ready
521+
tool_ctx = _get_ctx()
521522

522-
if not _is_deferred_recall:
523+
if not _skip_handler:
523524
handler_err = await _open_kitchen_handler()
524525
if handler_err is not None:
525526
return handler_err
527+
else:
528+
_ctx_post = _get_ctx()
529+
if _ctx_post.quota_refresh_task is None:
530+
try:
531+
_ctx_post.quota_refresh_task = create_background_task(
532+
_quota_refresh_loop(
533+
_ctx_post.config.quota_guard,
534+
provider=resolve_provider(_ctx_post.config.providers.default_provider),
535+
),
536+
label="quota_refresh_loop",
537+
)
538+
except Exception:
539+
logger.warning(
540+
"open_kitchen_quota_refresh_deferred_start_failed", exc_info=True
541+
)
526542

543+
if not _skip_handler:
527544
_kctx_pre = _get_ctx()
528545
_skip_notify = (
529546
_kctx_pre.backend is not None
@@ -538,6 +555,7 @@ async def open_kitchen(
538555
logger.warning(
539556
"open_kitchen_failure", stage="enable_components", exc_info=True
540557
)
558+
tool_ctx.gate_infrastructure_ready = False
541559
return _kitchen_failure_envelope(exc, stage="enable_components")
542560

543561
try:
@@ -550,8 +568,16 @@ async def open_kitchen(
550568
)
551569
except Exception as exc:
552570
logger.warning("open_kitchen_failure", stage="redisable_subsets", exc_info=True)
571+
tool_ctx.gate_infrastructure_ready = False
553572
return _kitchen_failure_envelope(exc, stage="redisable_subsets")
554573

574+
_is_deferred_recall = (
575+
name is not None
576+
and _ctx_pre.gate.enabled
577+
and _ctx_pre.recipe_name == name
578+
and _ctx_pre.recipe_name != ""
579+
)
580+
555581
_forbidden_list = ", ".join(PIPELINE_FORBIDDEN_TOOLS)
556582
_ctx = _get_ctx()
557583
_categories = _build_tool_category_listing(
@@ -645,6 +671,7 @@ async def open_kitchen(
645671
# Default to False for missing 'valid' so a absent key is treated as invalid
646672
if not result.get("valid", False) or not result.get("content", ""):
647673
tool_ctx.gate.disable()
674+
tool_ctx.gate_infrastructure_ready = False
648675
return _recipe_validation_error_response(name, result)
649676
# Dispatch-feasibility preflight: verify the backend can enforce
650677
# all fix-required hooks for the recipe's run_skill steps.
@@ -658,6 +685,7 @@ async def open_kitchen(
658685
)
659686
if _preflight_err is not None:
660687
tool_ctx.gate.disable()
688+
tool_ctx.gate_infrastructure_ready = False
661689
await ctx.disable_components(tags={"kitchen"})
662690
return _preflight_err
663691
result["success"] = True
@@ -680,6 +708,8 @@ async def open_kitchen(
680708
)
681709
if _override_warnings:
682710
result["warnings"] = _override_warnings
711+
if ingredients_only:
712+
result = strip_ingredients_only_keys(result)
683713
return json.dumps(result)
684714
try:
685715
result = tool_ctx.recipes.load_and_validate(
@@ -749,6 +779,7 @@ async def open_kitchen(
749779

750780
if not result.get("valid", False) or not result.get("content", ""):
751781
tool_ctx.gate.disable()
782+
tool_ctx.gate_infrastructure_ready = False
752783
return _recipe_validation_error_response(name, result)
753784

754785
# Dispatch-feasibility preflight: verify the backend can enforce
@@ -763,6 +794,7 @@ async def open_kitchen(
763794
)
764795
if _preflight_err is not None:
765796
tool_ctx.gate.disable()
797+
tool_ctx.gate_infrastructure_ready = False
766798
await ctx.disable_components(tags={"kitchen"})
767799
return _preflight_err
768800

tests/arch/test_subpackage_isolation.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -962,14 +962,16 @@ def test_data_directories_are_not_python_packages() -> None:
962962
"co-located with the execution engine that calls them",
963963
),
964964
"tools_kitchen.py": (
965-
1200,
965+
1250,
966966
"REQ-CNST-010-E7: kitchen tool handlers — open_kitchen and lock_ingredients require "
967967
"inline validation helpers (_check_override_keys, _build_ingredient_key_suggestions) "
968968
"for ingredient key validation; splitting would cross import-layer boundaries; "
969969
"backend capability promotion delegated to _promote_capability_keys in _auto_overrides; "
970970
"fail-closed validity gate on both deferred-recall and normal paths adds structural "
971971
"error propagation from LoadRecipeResult; get_recipe validity guard adds 9 lines; "
972-
"dispatch-feasibility preflight wiring on both paths with gate-close on failure",
972+
"dispatch-feasibility preflight wiring on both paths with gate-close on failure; "
973+
"gate_infrastructure_ready guard restructuring adds 22 lines for handler-skip path, "
974+
"quota_refresh_loop deferred start, and all four gate.disable() rollback resets",
973975
),
974976
"tools_execution.py": (
975977
1130,

tests/infra/test_schema_version_convention.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ def _is_yaml_dump(node: ast.expr) -> bool:
122122
("src/autoskillit/server/tools/tools_kitchen.py", 171),
123123
("src/autoskillit/server/tools/tools_kitchen.py", 190),
124124
("src/autoskillit/server/tools/tools_kitchen.py", 224),
125-
("src/autoskillit/server/tools/tools_kitchen.py", 918),
126-
("src/autoskillit/server/tools/tools_kitchen.py", 978),
125+
("src/autoskillit/server/tools/tools_kitchen.py", 950),
126+
("src/autoskillit/server/tools/tools_kitchen.py", 1010),
127127
# tools_pipeline_tracker.py — tracker_data dict
128128
("src/autoskillit/server/tools/tools_pipeline_tracker.py", 166),
129129
# tools_status.py — mcp_data dict

tests/server/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ def _make_mock_ctx() -> MagicMock:
107107
ctx.project_dir = Path("/fake/project")
108108
ctx.config.subsets.disabled = [] # REQ-VIS-008: no subsets disabled by default
109109
ctx.active_recipe_ingredients = None
110+
ctx.gate_infrastructure_ready = False
110111
return ctx
111112

112113

tests/server/test_kitchen_lifecycle.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,42 @@ async def test_close_kitchen_removes_tracker_dir(monkeypatch, tmp_path):
8787
_close_kitchen_handler()
8888

8989
assert not tracker_dir.exists()
90+
91+
92+
async def test_back_to_back_open_close_open_resets_infrastructure(monkeypatch, tmp_path):
93+
"""After close, gate_infrastructure_ready must be False so a re-open runs the handler."""
94+
monkeypatch.chdir(tmp_path)
95+
96+
ctx = make_context(
97+
AutomationConfig(),
98+
runner=None,
99+
plugin_source=DirectInstall(plugin_dir=tmp_path),
100+
project_dir=tmp_path,
101+
)
102+
monkeypatch.setattr(_state, "_ctx", ctx)
103+
monkeypatch.setattr(_state, "_startup_ready", None)
104+
105+
with (
106+
patch("autoskillit.server.tools.tools_kitchen._prime_quota_cache", new_callable=AsyncMock),
107+
patch("autoskillit.core.register_active_kitchen"),
108+
patch("autoskillit.core.unregister_active_kitchen"),
109+
):
110+
result1 = await _open_kitchen_handler()
111+
assert result1 is None
112+
assert ctx.gate_infrastructure_ready is True
113+
first_task = ctx.quota_refresh_task
114+
115+
_close_kitchen_handler()
116+
assert ctx.gate_infrastructure_ready is False
117+
await asyncio.sleep(0)
118+
assert first_task.cancelled() or first_task.done()
119+
120+
result2 = await _open_kitchen_handler()
121+
assert result2 is None
122+
assert ctx.gate_infrastructure_ready is True
123+
second_task = ctx.quota_refresh_task
124+
assert second_task is not first_task
125+
126+
_close_kitchen_handler()
127+
await asyncio.sleep(0)
128+
assert second_task.cancelled() or second_task.done()

tests/server/test_lifespan_skill_boot.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ async def test_codex_non_headless_pre_reveal_opens_gate(self, build_ctx, monkeyp
353353
assert ctx.kitchen_id is not None, "kitchen_id should be set by pre-reveal path"
354354
assert ctx.active_recipe_packs == frozenset()
355355
assert ctx.active_recipe_steps == {}
356+
assert ctx.gate_infrastructure_ready is True, (
357+
"gate_infrastructure_ready should be True after pre-reveal"
358+
)
356359

357360
@pytest.mark.anyio
358361
async def test_codex_non_headless_pre_reveal_suppresses_disabled_subset(

0 commit comments

Comments
 (0)