diff --git a/src/autoskillit/pipeline/context.py b/src/autoskillit/pipeline/context.py index e0ff16767..93d0dbc4e 100644 --- a/src/autoskillit/pipeline/context.py +++ b/src/autoskillit/pipeline/context.py @@ -168,6 +168,7 @@ class ToolContext: recipe_content_hash: str = field(default="") recipe_composite_hash: str = field(default="") recipe_version: str = field(default="") + gate_infrastructure_ready: bool = field(default=False) kitchen_id: str = field(default="") active_recipe_packs: frozenset[str] | None = field(default_factory=lambda: None) active_recipe_features: frozenset[str] | None = field(default_factory=lambda: None) diff --git a/src/autoskillit/server/_lifespan.py b/src/autoskillit/server/_lifespan.py index 26faaad75..98af204cc 100644 --- a/src/autoskillit/server/_lifespan.py +++ b/src/autoskillit/server/_lifespan.py @@ -342,6 +342,7 @@ async def _pre_reveal_kitchen(ctx: Any) -> None: register_active_kitchen(ctx.kitchen_id, os.getpid(), str(ctx.project_dir)) _write_hook_config() await _prime_quota_cache() + ctx.gate_infrastructure_ready = True async def _food_truck_auto_gate_boot(ctx: Any) -> None: diff --git a/src/autoskillit/server/tools/tools_kitchen.py b/src/autoskillit/server/tools/tools_kitchen.py index 5a673ab56..809a625b0 100644 --- a/src/autoskillit/server/tools/tools_kitchen.py +++ b/src/autoskillit/server/tools/tools_kitchen.py @@ -276,6 +276,7 @@ async def _open_kitchen_handler() -> str | None: except Exception: logger.warning("open_kitchen_registry_failed", exc_info=True) + ctx.gate_infrastructure_ready = True return None @@ -331,6 +332,7 @@ def _close_kitchen_handler() -> None: ctx.recipe_content_hash = "" ctx.recipe_composite_hash = "" ctx.recipe_version = "" + ctx.gate_infrastructure_ready = False logger.info("close_kitchen", gate_state="closed") if (log := ctx.github_api_log) is not None: orphan_usage = log.drain(ctx.kitchen_id) @@ -515,15 +517,30 @@ async def open_kitchen( disabled_subsets = _get_ctx().config.subsets.disabled _ctx_pre = _get_ctx() - _is_deferred_recall = ( - name is not None and _ctx_pre.gate.enabled and _ctx_pre.recipe_name == name - ) + _skip_handler = _ctx_pre.gate_infrastructure_ready + tool_ctx = _get_ctx() - if not _is_deferred_recall: + if not _skip_handler: handler_err = await _open_kitchen_handler() if handler_err is not None: return handler_err + else: + _ctx_post = _get_ctx() + if _ctx_post.quota_refresh_task is None: + try: + _ctx_post.quota_refresh_task = create_background_task( + _quota_refresh_loop( + _ctx_post.config.quota_guard, + provider=resolve_provider(_ctx_post.config.providers.default_provider), + ), + label="quota_refresh_loop", + ) + except Exception: + logger.warning( + "open_kitchen_quota_refresh_deferred_start_failed", exc_info=True + ) + if not _skip_handler: _kctx_pre = _get_ctx() _skip_notify = ( _kctx_pre.backend is not None @@ -538,6 +555,7 @@ async def open_kitchen( logger.warning( "open_kitchen_failure", stage="enable_components", exc_info=True ) + tool_ctx.gate_infrastructure_ready = False return _kitchen_failure_envelope(exc, stage="enable_components") try: @@ -550,8 +568,16 @@ async def open_kitchen( ) except Exception as exc: logger.warning("open_kitchen_failure", stage="redisable_subsets", exc_info=True) + tool_ctx.gate_infrastructure_ready = False return _kitchen_failure_envelope(exc, stage="redisable_subsets") + _is_deferred_recall = ( + name is not None + and _ctx_pre.gate.enabled + and _ctx_pre.recipe_name == name + and _ctx_pre.recipe_name != "" + ) + _forbidden_list = ", ".join(PIPELINE_FORBIDDEN_TOOLS) _ctx = _get_ctx() _categories = _build_tool_category_listing( @@ -645,6 +671,7 @@ async def open_kitchen( # Default to False for missing 'valid' so a absent key is treated as invalid if not result.get("valid", False) or not result.get("content", ""): tool_ctx.gate.disable() + tool_ctx.gate_infrastructure_ready = False return _recipe_validation_error_response(name, result) # Dispatch-feasibility preflight: verify the backend can enforce # all fix-required hooks for the recipe's run_skill steps. @@ -658,6 +685,7 @@ async def open_kitchen( ) if _preflight_err is not None: tool_ctx.gate.disable() + tool_ctx.gate_infrastructure_ready = False await ctx.disable_components(tags={"kitchen"}) return _preflight_err result["success"] = True @@ -680,6 +708,8 @@ async def open_kitchen( ) if _override_warnings: result["warnings"] = _override_warnings + if ingredients_only: + result = strip_ingredients_only_keys(result) return json.dumps(result) try: result = tool_ctx.recipes.load_and_validate( @@ -749,6 +779,7 @@ async def open_kitchen( if not result.get("valid", False) or not result.get("content", ""): tool_ctx.gate.disable() + tool_ctx.gate_infrastructure_ready = False return _recipe_validation_error_response(name, result) # Dispatch-feasibility preflight: verify the backend can enforce @@ -763,6 +794,7 @@ async def open_kitchen( ) if _preflight_err is not None: tool_ctx.gate.disable() + tool_ctx.gate_infrastructure_ready = False await ctx.disable_components(tags={"kitchen"}) return _preflight_err diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 1522a1cab..ea15a5a39 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -962,14 +962,16 @@ def test_data_directories_are_not_python_packages() -> None: "co-located with the execution engine that calls them", ), "tools_kitchen.py": ( - 1200, + 1250, "REQ-CNST-010-E7: kitchen tool handlers — open_kitchen and lock_ingredients require " "inline validation helpers (_check_override_keys, _build_ingredient_key_suggestions) " "for ingredient key validation; splitting would cross import-layer boundaries; " "backend capability promotion delegated to _promote_capability_keys in _auto_overrides; " "fail-closed validity gate on both deferred-recall and normal paths adds structural " "error propagation from LoadRecipeResult; get_recipe validity guard adds 9 lines; " - "dispatch-feasibility preflight wiring on both paths with gate-close on failure", + "dispatch-feasibility preflight wiring on both paths with gate-close on failure; " + "gate_infrastructure_ready guard restructuring adds 22 lines for handler-skip path, " + "quota_refresh_loop deferred start, and all four gate.disable() rollback resets", ), "tools_execution.py": ( 1130, diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index ebdd463a9..e33fa1f3b 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -122,8 +122,8 @@ def _is_yaml_dump(node: ast.expr) -> bool: ("src/autoskillit/server/tools/tools_kitchen.py", 171), ("src/autoskillit/server/tools/tools_kitchen.py", 190), ("src/autoskillit/server/tools/tools_kitchen.py", 224), - ("src/autoskillit/server/tools/tools_kitchen.py", 918), - ("src/autoskillit/server/tools/tools_kitchen.py", 978), + ("src/autoskillit/server/tools/tools_kitchen.py", 950), + ("src/autoskillit/server/tools/tools_kitchen.py", 1010), # tools_pipeline_tracker.py — tracker_data dict ("src/autoskillit/server/tools/tools_pipeline_tracker.py", 166), # tools_status.py — mcp_data dict diff --git a/tests/server/conftest.py b/tests/server/conftest.py index 1e93886cb..5753db1a0 100644 --- a/tests/server/conftest.py +++ b/tests/server/conftest.py @@ -107,6 +107,7 @@ def _make_mock_ctx() -> MagicMock: ctx.project_dir = Path("/fake/project") ctx.config.subsets.disabled = [] # REQ-VIS-008: no subsets disabled by default ctx.active_recipe_ingredients = None + ctx.gate_infrastructure_ready = False return ctx diff --git a/tests/server/test_kitchen_lifecycle.py b/tests/server/test_kitchen_lifecycle.py index c398a0f0e..dff3a6103 100644 --- a/tests/server/test_kitchen_lifecycle.py +++ b/tests/server/test_kitchen_lifecycle.py @@ -87,3 +87,42 @@ async def test_close_kitchen_removes_tracker_dir(monkeypatch, tmp_path): _close_kitchen_handler() assert not tracker_dir.exists() + + +async def test_back_to_back_open_close_open_resets_infrastructure(monkeypatch, tmp_path): + """After close, gate_infrastructure_ready must be False so a re-open runs the handler.""" + monkeypatch.chdir(tmp_path) + + ctx = make_context( + AutomationConfig(), + runner=None, + plugin_source=DirectInstall(plugin_dir=tmp_path), + project_dir=tmp_path, + ) + monkeypatch.setattr(_state, "_ctx", ctx) + monkeypatch.setattr(_state, "_startup_ready", None) + + with ( + patch("autoskillit.server.tools.tools_kitchen._prime_quota_cache", new_callable=AsyncMock), + patch("autoskillit.core.register_active_kitchen"), + patch("autoskillit.core.unregister_active_kitchen"), + ): + result1 = await _open_kitchen_handler() + assert result1 is None + assert ctx.gate_infrastructure_ready is True + first_task = ctx.quota_refresh_task + + _close_kitchen_handler() + assert ctx.gate_infrastructure_ready is False + await asyncio.sleep(0) + assert first_task.cancelled() or first_task.done() + + result2 = await _open_kitchen_handler() + assert result2 is None + assert ctx.gate_infrastructure_ready is True + second_task = ctx.quota_refresh_task + assert second_task is not first_task + + _close_kitchen_handler() + await asyncio.sleep(0) + assert second_task.cancelled() or second_task.done() diff --git a/tests/server/test_lifespan_skill_boot.py b/tests/server/test_lifespan_skill_boot.py index a6f5924f2..141ba9c68 100644 --- a/tests/server/test_lifespan_skill_boot.py +++ b/tests/server/test_lifespan_skill_boot.py @@ -353,6 +353,9 @@ async def test_codex_non_headless_pre_reveal_opens_gate(self, build_ctx, monkeyp assert ctx.kitchen_id is not None, "kitchen_id should be set by pre-reveal path" assert ctx.active_recipe_packs == frozenset() assert ctx.active_recipe_steps == {} + assert ctx.gate_infrastructure_ready is True, ( + "gate_infrastructure_ready should be True after pre-reveal" + ) @pytest.mark.anyio async def test_codex_non_headless_pre_reveal_suppresses_disabled_subset( diff --git a/tests/server/test_open_kitchen_deferred_recall.py b/tests/server/test_open_kitchen_deferred_recall.py index 3979e670c..8e0798c6a 100644 --- a/tests/server/test_open_kitchen_deferred_recall.py +++ b/tests/server/test_open_kitchen_deferred_recall.py @@ -4,7 +4,7 @@ import json from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -18,6 +18,7 @@ def _make_deferred_recall_ctx(name: str) -> MagicMock: ctx.gate.enabled = True ctx.recipe_name = name ctx.kitchen_id = "test-kitchen" + ctx.gate_infrastructure_ready = True return ctx @@ -196,3 +197,162 @@ async def test_deferred_recall_fails_closed_when_valid_missing(): assert parsed["stage"] == "recipe_validation" assert parsed["errors"] == [] assert "user_visible_message" in parsed + + +def _make_pre_revealed_ctx(name: str) -> MagicMock: + ctx = _make_mock_ctx() + ctx.gate.enabled = True + ctx.recipe_name = "" + ctx.kitchen_id = "test-kitchen" + ctx.gate_infrastructure_ready = True + ctx.recipes.load_and_validate.return_value = { + "content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n", + "valid": True, + "errors": [], + "requires_packs": [], + "requires_features": [], + "content_hash": "abc123", + "composite_hash": "def456", + "recipe_version": "1.0", + "suggestions": [], + "post_prune_step_names": ["build", "test"], + } + return ctx + + +@pytest.mark.anyio +async def test_pre_reveal_then_open_does_not_re_execute_handler(): + """Pre-revealed state (gate enabled, recipe_name empty, infrastructure ready) + must skip _open_kitchen_handler and still load the recipe.""" + from autoskillit.server.tools import tools_kitchen + + mock_ctx = _make_pre_revealed_ctx("test-recipe") + mock_recipe_info = MagicMock() + mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml") + mock_ctx.recipes.find.return_value = mock_recipe_info + mock_recipe_obj = MagicMock() + mock_recipe_obj.steps = {"build": {"cmd": "task build"}} + mock_recipe_obj.ingredients = {"ing1": "val1"} + mock_ctx.recipes.load.return_value = mock_recipe_obj + + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch.object( + tools_kitchen, "_open_kitchen_handler", new_callable=AsyncMock + ) as mock_handler, + ): + result = await tools_kitchen.open_kitchen(name="test-recipe", ctx=mock_ctx) + + mock_handler.assert_not_called() + parsed = json.loads(result) + assert parsed["success"] is True + + +@pytest.mark.anyio +async def test_deferred_recall_strips_content_when_ingredients_only_true(): + """Deferred-recall path must respect ingredients_only flag.""" + from autoskillit.server.tools import tools_kitchen + + mock_ctx = _make_deferred_recall_ctx("test-recipe") + mock_ctx.recipes.load_and_validate.return_value = { + "content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n", + "valid": True, + "errors": [], + "requires_packs": [], + "requires_features": [], + "content_hash": "abc", + "composite_hash": "def", + "recipe_version": "1.0", + "suggestions": [], + "orchestration_rules": "some rules", + "stop_step_semantics": "some semantics", + } + mock_recipe_info = MagicMock() + mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml") + mock_ctx.recipes.find.return_value = mock_recipe_info + mock_recipe_obj = MagicMock() + mock_recipe_obj.steps = {"build": {"cmd": "task build"}} + mock_recipe_obj.ingredients = {"ing1": "val1"} + mock_ctx.recipes.load.return_value = mock_recipe_obj + + with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + result = await tools_kitchen.open_kitchen( + name="test-recipe", ingredients_only=True, ctx=mock_ctx + ) + + parsed = json.loads(result) + assert "content" not in parsed + assert "orchestration_rules" not in parsed + assert "stop_step_semantics" not in parsed + + +@pytest.mark.anyio +async def test_double_open_kitchen_no_name_does_not_re_execute_handler(): + """Calling open_kitchen() with name=None while infrastructure is ready + must not re-run _open_kitchen_handler.""" + from autoskillit.server.tools import tools_kitchen + + mock_ctx = _make_mock_ctx() + mock_ctx.gate.enabled = True + mock_ctx.gate_infrastructure_ready = True + mock_ctx.kitchen_id = "test-kitchen" + + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch.object( + tools_kitchen, "_open_kitchen_handler", new_callable=AsyncMock + ) as mock_handler, + ): + result = await tools_kitchen.open_kitchen(ctx=mock_ctx) + + mock_handler.assert_not_called() + assert isinstance(result, str) + + +@pytest.mark.anyio +async def test_gate_rollback_resets_gate_infrastructure_ready(): + """When recipe validation fails in deferred-recall path, gate_infrastructure_ready + must be reset so the next open_kitchen call re-runs the handler.""" + from autoskillit.server.tools import tools_kitchen + + mock_ctx = _make_pre_revealed_ctx("bad-recipe") + mock_ctx.recipes.load_and_validate.return_value = { + "content": "", + "valid": False, + "errors": ["structural error"], + "requires_packs": [], + "requires_features": [], + "content_hash": "abc", + "composite_hash": "def", + "recipe_version": "1.0", + "suggestions": [], + } + + with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + result = await tools_kitchen.open_kitchen(name="bad-recipe", ctx=mock_ctx) + + parsed = json.loads(result) + assert parsed["success"] is False + assert mock_ctx.gate_infrastructure_ready is False + + +@pytest.mark.anyio +async def test_cold_open_kitchen_runs_handler(): + """When gate_infrastructure_ready is False (cold state), handler must run.""" + from autoskillit.server.tools import tools_kitchen + + mock_ctx = _make_mock_ctx() + mock_ctx.gate.enabled = False + mock_ctx.gate_infrastructure_ready = False + + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch.object( + tools_kitchen, "_open_kitchen_handler", new_callable=AsyncMock + ) as mock_handler, + ): + mock_handler.return_value = None + result = await tools_kitchen.open_kitchen(name="test-recipe", ctx=mock_ctx) + + mock_handler.assert_called_once() + assert isinstance(result, str)