diff --git a/src/autoskillit/config/ingredient_defaults.py b/src/autoskillit/config/ingredient_defaults.py index f18e75a22..25d8824b8 100644 --- a/src/autoskillit/config/ingredient_defaults.py +++ b/src/autoskillit/config/ingredient_defaults.py @@ -9,6 +9,7 @@ from typing import Protocol from autoskillit.core import ( + BACKEND_CAPABILITY_INGREDIENTS, CONFIG_AUTHORITY_KEYS, DISPATCH_ID_ENV_VAR, FLEET_MENU_TOOLS, @@ -205,18 +206,25 @@ def apply_config_authoritative_overrides( resolved = resolve_ingredient_defaults(project_dir) result = dict(effective_ingredients) for key in config_keys: - if key in resolved: - result[key] = resolved[key] - elif capability_overrides and key in capability_overrides: - # Fallback: only reached when resolve_ingredient_defaults() has no value for - # this key. Adding a key to resolve_ingredient_defaults shadows this branch. - result[key] = capability_overrides[key] - else: - logger.warning( - "config-authority key %r not found in resolved defaults — " - "caller-supplied value retained (config-authoritative contract not enforced)", - key, - ) + if key in SERVER_AUTHORITATIVE_INGREDIENTS: + if key in resolved: + result[key] = resolved[key] + else: + logger.warning( + "config-authority key %r not found in resolved defaults — " + "caller-supplied value retained (config-authoritative contract not enforced)", + key, + ) + elif key in BACKEND_CAPABILITY_INGREDIENTS: + if capability_overrides and key in capability_overrides: + result[key] = capability_overrides[key] + else: + logger.warning( + "capability-based key %r not found in capability_overrides — " + "caller-supplied value retained (capability contract not enforced)", + key, + ) + # else: key is caller-sovereign (e.g., source_dir) — leave result unchanged return result diff --git a/tests/config/test_helpers.py b/tests/config/test_helpers.py index 8b6cb825f..765dbeb1b 100644 --- a/tests/config/test_helpers.py +++ b/tests/config/test_helpers.py @@ -247,9 +247,10 @@ def test_apply_config_authoritative_overrides_capability_key_overrides_caller(tm assert result["backend_supports_git_write"] == "false" -def test_apply_config_authoritative_overrides_unknown_key_warns_and_retains(tmp_path): - """A config-authority key not in any registry (truly unknown) must trigger the - warning-and-retain path, not the capability override path.""" +def test_apply_config_authoritative_overrides_unknown_key_retains_caller_value(tmp_path): + """A config-authority key not in SERVER_AUTHORITATIVE_INGREDIENTS or + BACKEND_CAPABILITY_INGREDIENTS is caller-sovereign — the caller-supplied value + is retained silently (no warning).""" from types import SimpleNamespace import structlog.testing @@ -275,7 +276,7 @@ def test_apply_config_authoritative_overrides_unknown_key_warns_and_retains(tmp_ ) assert result["totally_unknown_key"] == "caller-value" - assert any("config-authority key" in e.get("event", "") for e in cap_logs) + assert not any("config-authority key" in e.get("event", "") for e in cap_logs) # T4: REQ-ING-003 diff --git a/tests/fleet/test_dispatch_ingredient_validation.py b/tests/fleet/test_dispatch_ingredient_validation.py index a5e60230f..27a643a3b 100644 --- a/tests/fleet/test_dispatch_ingredient_validation.py +++ b/tests/fleet/test_dispatch_ingredient_validation.py @@ -319,8 +319,9 @@ def _capture_prompt_builder(**kwargs): @pytest.mark.anyio async def test_config_authoritative_ingredients_injected_for_all_resolved_keys(self, tool_ctx): - """All ingredients declared authority='config' receive config values, - not just base_branch.""" + """SERVER_AUTHORITATIVE_INGREDIENTS keys with authority='config' receive config values; + source_dir with authority='config' is caller-sovereign and is NOT overridden from + resolved defaults.""" from unittest.mock import patch from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind @@ -373,7 +374,9 @@ def _capture_prompt_builder(**kwargs): ) assert captured["ingredients"]["base_branch"] == "develop" - assert captured["ingredients"]["source_dir"] == "/repo/src" + # source_dir is caller-sovereign — the URL from resolve_ingredient_defaults must NOT + # be injected even when the recipe declares authority="config". + assert captured["ingredients"].get("source_dir") != "/repo/src" assert captured["ingredients"]["local_review_rounds"] == "3" @pytest.mark.anyio @@ -431,12 +434,10 @@ async def test_config_authoritative_key_absent_from_defaults_retains_caller_valu self, tool_ctx ): """When a config-authority key is absent from resolved defaults AND not in - BACKEND_CAPABILITY_INGREDIENTS, the caller-supplied value is retained and a - warning is logged.""" + BACKEND_CAPABILITY_INGREDIENTS, the caller-supplied value is retained silently + (caller-sovereign path — no warning).""" from unittest.mock import patch - import structlog.testing - from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind _setup_config_authority_recipe( @@ -458,30 +459,31 @@ def _capture_prompt_builder(**kwargs): captured.update(kwargs) return "prompt" + import structlog.testing + from autoskillit.fleet._api import execute_dispatch - with structlog.testing.capture_logs() as cap_logs: - with patch( + with ( + patch( "autoskillit.config.ingredient_defaults.resolve_ingredient_defaults", return_value={}, # key absent from all registries - ): - await execute_dispatch( - tool_ctx=tool_ctx, - recipe="test-recipe", - task="t", - ingredients={"truly_unknown_key": "caller-supplied"}, - dispatch_name=None, - timeout_sec=None, - prompt_builder=_capture_prompt_builder, - quota_checker=_no_sleep_quota_checker, - quota_refresher=_noop_quota_refresher, - ) + ), + structlog.testing.capture_logs() as cap_logs, + ): + await execute_dispatch( + tool_ctx=tool_ctx, + recipe="test-recipe", + task="t", + ingredients={"truly_unknown_key": "caller-supplied"}, + dispatch_name=None, + timeout_sec=None, + prompt_builder=_capture_prompt_builder, + quota_checker=_no_sleep_quota_checker, + quota_refresher=_noop_quota_refresher, + ) assert captured["ingredients"]["truly_unknown_key"] == "caller-supplied" - assert any( - e.get("log_level") == "warning" and "config-authority key" in e.get("event", "") - for e in cap_logs - ) + assert not any("config-authority key" in e.get("event", "") for e in cap_logs) @pytest.mark.anyio async def test_non_writable_backend_forces_git_write_false_at_dispatch(self, tool_ctx): @@ -537,6 +539,125 @@ def _capture_prompt_builder(**kwargs): assert captured["ingredients"]["backend_supports_git_write"] == "false" + @pytest.mark.anyio + async def test_apply_config_authoritative_overrides_source_dir_preserves_caller_local_path( + self, tool_ctx + ): + """When the recipe declares source_dir with authority='config' and the caller + supplies a local path, apply_config_authoritative_overrides must preserve the + caller's value — source_dir is caller-sovereign project identity.""" + from unittest.mock import patch + + from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind + + _setup_config_authority_recipe( + tool_ctx, + Recipe( + name="test-recipe", + description="test", + kind=RecipeKind.STANDARD, + ingredients={ + "source_dir": RecipeIngredient( + description="Source directory", default="", authority="config" + ), + }, + ), + ) + captured: dict = {} + + def _capture_prompt_builder(**kwargs): + captured.update(kwargs) + return "prompt" + + from autoskillit.fleet._api import execute_dispatch + + with patch( + "autoskillit.config.ingredient_defaults.resolve_ingredient_defaults", + return_value={ + "source_dir": "https://github.com/TalonT-Org/AutoSkillit", + "base_branch": "main", + }, + ): + await execute_dispatch( + tool_ctx=tool_ctx, + recipe="test-recipe", + task="t", + ingredients={"source_dir": "/home/user/myproject"}, + dispatch_name=None, + timeout_sec=None, + prompt_builder=_capture_prompt_builder, + quota_checker=_no_sleep_quota_checker, + quota_refresher=_noop_quota_refresher, + ) + + assert captured["ingredients"]["source_dir"] == "/home/user/myproject" + + def test_apply_config_authoritative_overrides_source_dir_not_injected_when_absent( + self, tmp_path + ): + """When the caller does not supply source_dir, apply_config_authoritative_overrides + must not inject the URL returned by resolve_ingredient_defaults — source_dir is + caller-sovereign and must remain absent from the result.""" + from types import SimpleNamespace + from unittest.mock import patch + + from autoskillit.config import apply_config_authoritative_overrides + + recipe_ingredients = { + "source_dir": SimpleNamespace(authority="config"), + } + with patch( + "autoskillit.config.ingredient_defaults.resolve_ingredient_defaults", + return_value={"source_dir": "https://github.com/TalonT-Org/AutoSkillit"}, + ): + result = apply_config_authoritative_overrides( + {}, + recipe_ingredients, + tmp_path, + ) + + assert "source_dir" not in result + + def test_apply_config_authoritative_overrides_only_mutates_enforcement_keys(self, tmp_path): + """For a recipe that declares ALL CONFIG_AUTHORITY_KEYS as authority='config', + the function must only mutate keys in SERVER_AUTHORITATIVE_INGREDIENTS | + BACKEND_CAPABILITY_INGREDIENTS. source_dir is caller-sovereign and must not + appear in the changed-key set.""" + from types import SimpleNamespace + from unittest.mock import patch + + from autoskillit.config import apply_config_authoritative_overrides + from autoskillit.config.ingredient_defaults import ( + BACKEND_CAPABILITY_INGREDIENTS, + SERVER_AUTHORITATIVE_INGREDIENTS, + ) + from autoskillit.core import CONFIG_AUTHORITY_KEYS + + recipe_ingredients = { + key: SimpleNamespace(authority="config") for key in CONFIG_AUTHORITY_KEYS + } + resolved_values = {key: f"resolved-{key}" for key in CONFIG_AUTHORITY_KEYS} + capability_values = {key: f"cap-{key}" for key in BACKEND_CAPABILITY_INGREDIENTS} + + with patch( + "autoskillit.config.ingredient_defaults.resolve_ingredient_defaults", + return_value=resolved_values, + ): + result = apply_config_authoritative_overrides( + {}, + recipe_ingredients, + tmp_path, + capability_overrides=capability_values, + ) + + changed_keys = set(result.keys()) + assert changed_keys <= ( + SERVER_AUTHORITATIVE_INGREDIENTS | BACKEND_CAPABILITY_INGREDIENTS + ), ( + f"apply_config_authoritative_overrides mutated caller-sovereign keys: " + f"{changed_keys - (SERVER_AUTHORITATIVE_INGREDIENTS | BACKEND_CAPABILITY_INGREDIENTS)}" + ) + def test_capability_overrides_dict_covers_registry_keys(): """Structural test: every key in BACKEND_CAPABILITY_INGREDIENTS must appear in the