Skip to content

Commit 47e8983

Browse files
Trecekclaude
andcommitted
[FIX] Double open_kitchen Context Cost — gate_infrastructure_ready immunity
Pre-reveal state (Codex backend) sets gate.enabled=True but leaves recipe_name="". The first open_kitchen(name="X") call failed the _is_deferred_recall guard, re-executing _open_kitchen_handler() and re-sending the full ~100KB recipe payload. This fix adds a gate_infrastructure_ready boolean to ToolContext that tracks whether gate infrastructure has been initialized (by either _pre_reveal_kitchen or _open_kitchen_handler). The handler is now skipped when this flag is True, decoupled from recipe name matching. Changes: - Add gate_infrastructure_ready field to ToolContext - Set flag in _open_kitchen_handler and _pre_reveal_kitchen - Replace _is_deferred_recall handler-skip with gate_infrastructure_ready check - Reset flag in _close_kitchen_handler and on all gate.disable() rollback sites - Start quota_refresh_loop conditionally after handler-skip - Apply ingredients_only stripping in deferred-recall path - Guard _skip_notify block with _skip_handler (Step 3a) - Add tests: pre-reveal skip, ingredients_only strip, double no-name, rollback reset, backend parametrize, close-then-reopen Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0b8dc6c commit 47e8983

7 files changed

Lines changed: 262 additions & 4 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: 33 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,29 @@ 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
521521

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

542+
if not _skip_handler:
527543
_kctx_pre = _get_ctx()
528544
_skip_notify = (
529545
_kctx_pre.backend is not None
@@ -552,6 +568,13 @@ async def open_kitchen(
552568
logger.warning("open_kitchen_failure", stage="redisable_subsets", exc_info=True)
553569
return _kitchen_failure_envelope(exc, stage="redisable_subsets")
554570

571+
_is_deferred_recall = (
572+
name is not None
573+
and _ctx_pre.gate.enabled
574+
and _ctx_pre.recipe_name == name
575+
and _ctx_pre.recipe_name != ""
576+
)
577+
555578
_forbidden_list = ", ".join(PIPELINE_FORBIDDEN_TOOLS)
556579
_ctx = _get_ctx()
557580
_categories = _build_tool_category_listing(
@@ -645,6 +668,7 @@ async def open_kitchen(
645668
# Default to False for missing 'valid' so a absent key is treated as invalid
646669
if not result.get("valid", False) or not result.get("content", ""):
647670
tool_ctx.gate.disable()
671+
tool_ctx.gate_infrastructure_ready = False
648672
return _recipe_validation_error_response(name, result)
649673
# Dispatch-feasibility preflight: verify the backend can enforce
650674
# all fix-required hooks for the recipe's run_skill steps.
@@ -658,6 +682,7 @@ async def open_kitchen(
658682
)
659683
if _preflight_err is not None:
660684
tool_ctx.gate.disable()
685+
tool_ctx.gate_infrastructure_ready = False
661686
await ctx.disable_components(tags={"kitchen"})
662687
return _preflight_err
663688
result["success"] = True
@@ -680,6 +705,8 @@ async def open_kitchen(
680705
)
681706
if _override_warnings:
682707
result["warnings"] = _override_warnings
708+
if ingredients_only:
709+
result = strip_ingredients_only_keys(result)
683710
return json.dumps(result)
684711
try:
685712
result = tool_ctx.recipes.load_and_validate(
@@ -749,6 +776,7 @@ async def open_kitchen(
749776

750777
if not result.get("valid", False) or not result.get("content", ""):
751778
tool_ctx.gate.disable()
779+
tool_ctx.gate_infrastructure_ready = False
752780
return _recipe_validation_error_response(name, result)
753781

754782
# Dispatch-feasibility preflight: verify the backend can enforce
@@ -763,6 +791,7 @@ async def open_kitchen(
763791
)
764792
if _preflight_err is not None:
765793
tool_ctx.gate.disable()
794+
tool_ctx.gate_infrastructure_ready = False
766795
await ctx.disable_components(tags={"kitchen"})
767796
return _preflight_err
768797

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: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,38 @@ 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+
114+
_close_kitchen_handler()
115+
assert ctx.gate_infrastructure_ready is False
116+
117+
result2 = await _open_kitchen_handler()
118+
assert result2 is None
119+
assert ctx.gate_infrastructure_ready is True
120+
121+
task = ctx.quota_refresh_task
122+
await asyncio.sleep(0)
123+
if task is not None:
124+
assert task.cancelled() or 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(

tests/server/test_open_kitchen_deferred_recall.py

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ def _make_deferred_recall_ctx(name: str) -> MagicMock:
1818
ctx.gate.enabled = True
1919
ctx.recipe_name = name
2020
ctx.kitchen_id = "test-kitchen"
21+
ctx.gate_infrastructure_ready = True
2122
return ctx
2223

2324

@@ -196,3 +197,190 @@ async def test_deferred_recall_fails_closed_when_valid_missing():
196197
assert parsed["stage"] == "recipe_validation"
197198
assert parsed["errors"] == []
198199
assert "user_visible_message" in parsed
200+
201+
202+
def _make_pre_revealed_ctx(name: str) -> MagicMock:
203+
ctx = _make_mock_ctx()
204+
ctx.gate.enabled = True
205+
ctx.recipe_name = ""
206+
ctx.kitchen_id = "test-kitchen"
207+
ctx.gate_infrastructure_ready = True
208+
ctx.recipes.load_and_validate.return_value = {
209+
"content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n",
210+
"valid": True,
211+
"errors": [],
212+
"requires_packs": [],
213+
"requires_features": [],
214+
"content_hash": "abc123",
215+
"composite_hash": "def456",
216+
"recipe_version": "1.0",
217+
"suggestions": [],
218+
"post_prune_step_names": ["build", "test"],
219+
}
220+
return ctx
221+
222+
223+
@pytest.mark.anyio
224+
async def test_pre_reveal_then_open_does_not_re_execute_handler():
225+
"""Pre-revealed state (gate enabled, recipe_name empty, infrastructure ready)
226+
must skip _open_kitchen_handler and still load the recipe."""
227+
from autoskillit.server.tools import tools_kitchen
228+
229+
mock_ctx = _make_pre_revealed_ctx("test-recipe")
230+
mock_recipe_info = MagicMock()
231+
mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml")
232+
mock_ctx.recipes.find.return_value = mock_recipe_info
233+
mock_recipe_obj = MagicMock()
234+
mock_recipe_obj.steps = {"build": {"cmd": "task build"}}
235+
mock_recipe_obj.ingredients = {"ing1": "val1"}
236+
mock_ctx.recipes.load.return_value = mock_recipe_obj
237+
238+
with (
239+
patch("autoskillit.server._get_ctx", return_value=mock_ctx),
240+
patch.object(
241+
tools_kitchen, "_open_kitchen_handler", new_callable=MagicMock
242+
) as mock_handler,
243+
):
244+
result = await tools_kitchen.open_kitchen(name="test-recipe", ctx=mock_ctx)
245+
246+
mock_handler.assert_not_called()
247+
parsed = json.loads(result)
248+
assert parsed["success"] is True
249+
250+
251+
@pytest.mark.anyio
252+
async def test_deferred_recall_strips_content_when_ingredients_only_true():
253+
"""Deferred-recall path must respect ingredients_only flag."""
254+
from autoskillit.server.tools import tools_kitchen
255+
256+
mock_ctx = _make_deferred_recall_ctx("test-recipe")
257+
mock_ctx.recipes.load_and_validate.return_value = {
258+
"content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n",
259+
"valid": True,
260+
"errors": [],
261+
"requires_packs": [],
262+
"requires_features": [],
263+
"content_hash": "abc",
264+
"composite_hash": "def",
265+
"recipe_version": "1.0",
266+
"suggestions": [],
267+
"orchestration_rules": "some rules",
268+
"stop_step_semantics": "some semantics",
269+
}
270+
mock_recipe_info = MagicMock()
271+
mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml")
272+
mock_ctx.recipes.find.return_value = mock_recipe_info
273+
mock_recipe_obj = MagicMock()
274+
mock_recipe_obj.steps = {"build": {"cmd": "task build"}}
275+
mock_recipe_obj.ingredients = {"ing1": "val1"}
276+
mock_ctx.recipes.load.return_value = mock_recipe_obj
277+
278+
with patch("autoskillit.server._get_ctx", return_value=mock_ctx):
279+
result = await tools_kitchen.open_kitchen(
280+
name="test-recipe", ingredients_only=True, ctx=mock_ctx
281+
)
282+
283+
parsed = json.loads(result)
284+
assert "content" not in parsed
285+
assert "orchestration_rules" not in parsed
286+
assert "stop_step_semantics" not in parsed
287+
288+
289+
@pytest.mark.anyio
290+
async def test_double_open_kitchen_no_name_does_not_re_execute_handler():
291+
"""Calling open_kitchen() with name=None while infrastructure is ready
292+
must not re-run _open_kitchen_handler."""
293+
from autoskillit.server.tools import tools_kitchen
294+
295+
mock_ctx = _make_mock_ctx()
296+
mock_ctx.gate.enabled = True
297+
mock_ctx.gate_infrastructure_ready = True
298+
mock_ctx.kitchen_id = "test-kitchen"
299+
300+
with (
301+
patch("autoskillit.server._get_ctx", return_value=mock_ctx),
302+
patch.object(
303+
tools_kitchen, "_open_kitchen_handler", new_callable=MagicMock
304+
) as mock_handler,
305+
):
306+
result = await tools_kitchen.open_kitchen(ctx=mock_ctx)
307+
308+
mock_handler.assert_not_called()
309+
assert isinstance(result, str)
310+
311+
312+
@pytest.mark.anyio
313+
async def test_gate_rollback_resets_gate_infrastructure_ready():
314+
"""When recipe validation fails in deferred-recall path, gate_infrastructure_ready
315+
must be reset so the next open_kitchen call re-runs the handler."""
316+
from autoskillit.server.tools import tools_kitchen
317+
318+
mock_ctx = _make_pre_revealed_ctx("bad-recipe")
319+
mock_ctx.recipes.load_and_validate.return_value = {
320+
"content": "",
321+
"valid": False,
322+
"errors": ["structural error"],
323+
"requires_packs": [],
324+
"requires_features": [],
325+
"content_hash": "abc",
326+
"composite_hash": "def",
327+
"recipe_version": "1.0",
328+
"suggestions": [],
329+
}
330+
331+
with patch("autoskillit.server._get_ctx", return_value=mock_ctx):
332+
result = await tools_kitchen.open_kitchen(name="bad-recipe", ctx=mock_ctx)
333+
334+
parsed = json.loads(result)
335+
assert parsed["success"] is False
336+
assert mock_ctx.gate_infrastructure_ready is False
337+
338+
339+
@pytest.mark.anyio
340+
async def test_pre_reveal_backend_does_not_support_tool_list_changed():
341+
"""Simulates Codex pre-reveal: gate enabled, recipe_name empty, infrastructure ready.
342+
Handler must be skipped, recipe loaded normally."""
343+
from autoskillit.server.tools import tools_kitchen
344+
345+
mock_ctx = _make_pre_revealed_ctx("test-recipe")
346+
mock_recipe_info = MagicMock()
347+
mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml")
348+
mock_ctx.recipes.find.return_value = mock_recipe_info
349+
mock_recipe_obj = MagicMock()
350+
mock_recipe_obj.steps = {"build": {"cmd": "task build"}}
351+
mock_recipe_obj.ingredients = {"ing1": "val1"}
352+
mock_ctx.recipes.load.return_value = mock_recipe_obj
353+
354+
with (
355+
patch("autoskillit.server._get_ctx", return_value=mock_ctx),
356+
patch.object(
357+
tools_kitchen, "_open_kitchen_handler", new_callable=MagicMock
358+
) as mock_handler,
359+
):
360+
result = await tools_kitchen.open_kitchen(name="test-recipe", ctx=mock_ctx)
361+
362+
mock_handler.assert_not_called()
363+
parsed = json.loads(result)
364+
assert parsed["success"] is True
365+
366+
367+
@pytest.mark.anyio
368+
async def test_cold_open_kitchen_runs_handler():
369+
"""When gate_infrastructure_ready is False (cold state), handler must run."""
370+
from autoskillit.server.tools import tools_kitchen
371+
372+
mock_ctx = _make_mock_ctx()
373+
mock_ctx.gate.enabled = False
374+
mock_ctx.gate_infrastructure_ready = False
375+
376+
with (
377+
patch("autoskillit.server._get_ctx", return_value=mock_ctx),
378+
patch.object(
379+
tools_kitchen, "_open_kitchen_handler", new_callable=MagicMock
380+
) as mock_handler,
381+
):
382+
mock_handler.return_value = None
383+
result = await tools_kitchen.open_kitchen(name="test-recipe", ctx=mock_ctx)
384+
385+
mock_handler.assert_called_once()
386+
assert isinstance(result, str)

0 commit comments

Comments
 (0)