Skip to content

Commit 46519ee

Browse files
Trecekclaude
andauthored
Implementation Plan: Remediation — source-dir-enforcement-seam Part A (#4201)
## Summary Two concrete code defects were left behind by the prior implementation round and are cited in the remediation audit: 1. **REQ-004 — unauthorized fourth branch**: `apply_config_authoritative_overrides` in `src/autoskillit/config/ingredient_defaults.py` contains a fourth `elif key not in CONFIG_AUTHORITY_KEYS: logger.warning(...)` branch that the plan never authorized. The plan specifies exactly three branches: `SERVER_AUTHORITATIVE_INGREDIENTS`, `BACKEND_CAPABILITY_INGREDIENTS`, and an implicit `else` (caller-sovereign). The fourth branch must be removed so the loop body matches the specified three-branch structure. 2. **REQ-009 — weak T2 assertion**: The T2 test `test_apply_config_authoritative_overrides_source_dir_not_injected_when_absent` uses `captured["ingredients"].get("source_dir") != URL` — a weaker assertion that passes if `source_dir` is present with any non-URL value (e.g., the empty string `""` injected by `execute_dispatch` when the caller omits the ingredient and the recipe defines `default=""`). The test must assert `"source_dir" not in result` against the output of `apply_config_authoritative_overrides` directly, bypassing the `execute_dispatch` default-injection path. Both changes are in the same commit to satisfy the green-gate invariant (the T2 test must pass after the function fix, and the function fix changes the behavior T2 validates). Part B (introducing `CALLER_SOVEREIGN_INGREDIENTS`, tightening config-authority-requires-resolve-source, removing `authority: config` from 15 recipe YAMLs) is a separate task and must NOT be implemented here. Closes #4200 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260707-023904-630849/.autoskillit/temp/make-plan/remediation_source-dir-enforcement-seam-part-a_plan_2026-07-07_035458.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 | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | investigate* | fable | 1 | 8.9k | 34.4k | 769.5k | 126.2k | 27 | 136.5k | 24m 45s | | rectify* | sonnet | 1 | 219 | 38.1k | 2.1M | 118.9k | 72 | 119.6k | 20m 17s | | dry_walkthrough* | sonnet | 2 | 1.7k | 24.8k | 924.2k | 61.7k | 54 | 85.0k | 10m 12s | | implement* | sonnet | 2 | 290 | 18.8k | 1.8M | 77.2k | 87 | 96.3k | 8m 32s | | assess* | sonnet | 3 | 354 | 23.0k | 2.0M | 66.4k | 108 | 118.0k | 15m 5s | | audit_impl* | sonnet | 2 | 194 | 45.7k | 1.0M | 72.0k | 65 | 108.0k | 16m 16s | | make_plan* | sonnet | 1 | 103 | 10.0k | 577.4k | 63.8k | 30 | 49.4k | 3m 17s | | prepare_pr* | MiniMax-M3 | 1 | 42.2k | 2.6k | 153.7k | 44.1k | 12 | 0 | 43s | | compose_pr* | MiniMax-M3 | 1 | 35.7k | 2.5k | 182.9k | 0 | 13 | 0 | 37s | | **Total** | | | 89.6k | 200.0k | 9.6M | 126.2k | | 712.9k | 1h 39m | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | investigate | 0 | — | — | — | | rectify | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 250 | 7150.7 | 385.4 | 75.0 | | assess | 66 | 30170.3 | 1787.4 | 349.0 | | audit_impl | 0 | — | — | — | | make_plan | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **316** | 30286.9 | 2255.9 | 632.8 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | fable | 1 | 8.9k | 34.4k | 769.5k | 136.5k | 24m 45s | | sonnet | 6 | 2.9k | 160.5k | 8.5M | 576.3k | 1h 13m | | MiniMax-M3 | 2 | 77.9k | 5.1k | 336.6k | 0 | 1m 20s | --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 704a8cc commit 46519ee

3 files changed

Lines changed: 171 additions & 41 deletions

File tree

src/autoskillit/config/ingredient_defaults.py

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from typing import Protocol
1010

1111
from autoskillit.core import (
12+
BACKEND_CAPABILITY_INGREDIENTS,
1213
CONFIG_AUTHORITY_KEYS,
1314
DISPATCH_ID_ENV_VAR,
1415
FLEET_MENU_TOOLS,
@@ -205,18 +206,25 @@ def apply_config_authoritative_overrides(
205206
resolved = resolve_ingredient_defaults(project_dir)
206207
result = dict(effective_ingredients)
207208
for key in config_keys:
208-
if key in resolved:
209-
result[key] = resolved[key]
210-
elif capability_overrides and key in capability_overrides:
211-
# Fallback: only reached when resolve_ingredient_defaults() has no value for
212-
# this key. Adding a key to resolve_ingredient_defaults shadows this branch.
213-
result[key] = capability_overrides[key]
214-
else:
215-
logger.warning(
216-
"config-authority key %r not found in resolved defaults — "
217-
"caller-supplied value retained (config-authoritative contract not enforced)",
218-
key,
219-
)
209+
if key in SERVER_AUTHORITATIVE_INGREDIENTS:
210+
if key in resolved:
211+
result[key] = resolved[key]
212+
else:
213+
logger.warning(
214+
"config-authority key %r not found in resolved defaults — "
215+
"caller-supplied value retained (config-authoritative contract not enforced)",
216+
key,
217+
)
218+
elif key in BACKEND_CAPABILITY_INGREDIENTS:
219+
if capability_overrides and key in capability_overrides:
220+
result[key] = capability_overrides[key]
221+
else:
222+
logger.warning(
223+
"capability-based key %r not found in capability_overrides — "
224+
"caller-supplied value retained (capability contract not enforced)",
225+
key,
226+
)
227+
# else: key is caller-sovereign (e.g., source_dir) — leave result unchanged
220228
return result
221229

222230

tests/config/test_helpers.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,10 @@ def test_apply_config_authoritative_overrides_capability_key_overrides_caller(tm
247247
assert result["backend_supports_git_write"] == "false"
248248

249249

250-
def test_apply_config_authoritative_overrides_unknown_key_warns_and_retains(tmp_path):
251-
"""A config-authority key not in any registry (truly unknown) must trigger the
252-
warning-and-retain path, not the capability override path."""
250+
def test_apply_config_authoritative_overrides_unknown_key_retains_caller_value(tmp_path):
251+
"""A config-authority key not in SERVER_AUTHORITATIVE_INGREDIENTS or
252+
BACKEND_CAPABILITY_INGREDIENTS is caller-sovereign — the caller-supplied value
253+
is retained silently (no warning)."""
253254
from types import SimpleNamespace
254255

255256
import structlog.testing
@@ -275,7 +276,7 @@ def test_apply_config_authoritative_overrides_unknown_key_warns_and_retains(tmp_
275276
)
276277

277278
assert result["totally_unknown_key"] == "caller-value"
278-
assert any("config-authority key" in e.get("event", "") for e in cap_logs)
279+
assert not any("config-authority key" in e.get("event", "") for e in cap_logs)
279280

280281

281282
# T4: REQ-ING-003

tests/fleet/test_dispatch_ingredient_validation.py

Lines changed: 146 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,9 @@ def _capture_prompt_builder(**kwargs):
319319

320320
@pytest.mark.anyio
321321
async def test_config_authoritative_ingredients_injected_for_all_resolved_keys(self, tool_ctx):
322-
"""All ingredients declared authority='config' receive config values,
323-
not just base_branch."""
322+
"""SERVER_AUTHORITATIVE_INGREDIENTS keys with authority='config' receive config values;
323+
source_dir with authority='config' is caller-sovereign and is NOT overridden from
324+
resolved defaults."""
324325
from unittest.mock import patch
325326

326327
from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind
@@ -373,7 +374,9 @@ def _capture_prompt_builder(**kwargs):
373374
)
374375

375376
assert captured["ingredients"]["base_branch"] == "develop"
376-
assert captured["ingredients"]["source_dir"] == "/repo/src"
377+
# source_dir is caller-sovereign — the URL from resolve_ingredient_defaults must NOT
378+
# be injected even when the recipe declares authority="config".
379+
assert captured["ingredients"].get("source_dir") != "/repo/src"
377380
assert captured["ingredients"]["local_review_rounds"] == "3"
378381

379382
@pytest.mark.anyio
@@ -431,12 +434,10 @@ async def test_config_authoritative_key_absent_from_defaults_retains_caller_valu
431434
self, tool_ctx
432435
):
433436
"""When a config-authority key is absent from resolved defaults AND not in
434-
BACKEND_CAPABILITY_INGREDIENTS, the caller-supplied value is retained and a
435-
warning is logged."""
437+
BACKEND_CAPABILITY_INGREDIENTS, the caller-supplied value is retained silently
438+
(caller-sovereign path — no warning)."""
436439
from unittest.mock import patch
437440

438-
import structlog.testing
439-
440441
from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind
441442

442443
_setup_config_authority_recipe(
@@ -458,30 +459,31 @@ def _capture_prompt_builder(**kwargs):
458459
captured.update(kwargs)
459460
return "prompt"
460461

462+
import structlog.testing
463+
461464
from autoskillit.fleet._api import execute_dispatch
462465

463-
with structlog.testing.capture_logs() as cap_logs:
464-
with patch(
466+
with (
467+
patch(
465468
"autoskillit.config.ingredient_defaults.resolve_ingredient_defaults",
466469
return_value={}, # key absent from all registries
467-
):
468-
await execute_dispatch(
469-
tool_ctx=tool_ctx,
470-
recipe="test-recipe",
471-
task="t",
472-
ingredients={"truly_unknown_key": "caller-supplied"},
473-
dispatch_name=None,
474-
timeout_sec=None,
475-
prompt_builder=_capture_prompt_builder,
476-
quota_checker=_no_sleep_quota_checker,
477-
quota_refresher=_noop_quota_refresher,
478-
)
470+
),
471+
structlog.testing.capture_logs() as cap_logs,
472+
):
473+
await execute_dispatch(
474+
tool_ctx=tool_ctx,
475+
recipe="test-recipe",
476+
task="t",
477+
ingredients={"truly_unknown_key": "caller-supplied"},
478+
dispatch_name=None,
479+
timeout_sec=None,
480+
prompt_builder=_capture_prompt_builder,
481+
quota_checker=_no_sleep_quota_checker,
482+
quota_refresher=_noop_quota_refresher,
483+
)
479484

480485
assert captured["ingredients"]["truly_unknown_key"] == "caller-supplied"
481-
assert any(
482-
e.get("log_level") == "warning" and "config-authority key" in e.get("event", "")
483-
for e in cap_logs
484-
)
486+
assert not any("config-authority key" in e.get("event", "") for e in cap_logs)
485487

486488
@pytest.mark.anyio
487489
async def test_non_writable_backend_forces_git_write_false_at_dispatch(self, tool_ctx):
@@ -537,6 +539,125 @@ def _capture_prompt_builder(**kwargs):
537539

538540
assert captured["ingredients"]["backend_supports_git_write"] == "false"
539541

542+
@pytest.mark.anyio
543+
async def test_apply_config_authoritative_overrides_source_dir_preserves_caller_local_path(
544+
self, tool_ctx
545+
):
546+
"""When the recipe declares source_dir with authority='config' and the caller
547+
supplies a local path, apply_config_authoritative_overrides must preserve the
548+
caller's value — source_dir is caller-sovereign project identity."""
549+
from unittest.mock import patch
550+
551+
from autoskillit.recipe.schema import Recipe, RecipeIngredient, RecipeKind
552+
553+
_setup_config_authority_recipe(
554+
tool_ctx,
555+
Recipe(
556+
name="test-recipe",
557+
description="test",
558+
kind=RecipeKind.STANDARD,
559+
ingredients={
560+
"source_dir": RecipeIngredient(
561+
description="Source directory", default="", authority="config"
562+
),
563+
},
564+
),
565+
)
566+
captured: dict = {}
567+
568+
def _capture_prompt_builder(**kwargs):
569+
captured.update(kwargs)
570+
return "prompt"
571+
572+
from autoskillit.fleet._api import execute_dispatch
573+
574+
with patch(
575+
"autoskillit.config.ingredient_defaults.resolve_ingredient_defaults",
576+
return_value={
577+
"source_dir": "https://github.com/TalonT-Org/AutoSkillit",
578+
"base_branch": "main",
579+
},
580+
):
581+
await execute_dispatch(
582+
tool_ctx=tool_ctx,
583+
recipe="test-recipe",
584+
task="t",
585+
ingredients={"source_dir": "/home/user/myproject"},
586+
dispatch_name=None,
587+
timeout_sec=None,
588+
prompt_builder=_capture_prompt_builder,
589+
quota_checker=_no_sleep_quota_checker,
590+
quota_refresher=_noop_quota_refresher,
591+
)
592+
593+
assert captured["ingredients"]["source_dir"] == "/home/user/myproject"
594+
595+
def test_apply_config_authoritative_overrides_source_dir_not_injected_when_absent(
596+
self, tmp_path
597+
):
598+
"""When the caller does not supply source_dir, apply_config_authoritative_overrides
599+
must not inject the URL returned by resolve_ingredient_defaults — source_dir is
600+
caller-sovereign and must remain absent from the result."""
601+
from types import SimpleNamespace
602+
from unittest.mock import patch
603+
604+
from autoskillit.config import apply_config_authoritative_overrides
605+
606+
recipe_ingredients = {
607+
"source_dir": SimpleNamespace(authority="config"),
608+
}
609+
with patch(
610+
"autoskillit.config.ingredient_defaults.resolve_ingredient_defaults",
611+
return_value={"source_dir": "https://github.com/TalonT-Org/AutoSkillit"},
612+
):
613+
result = apply_config_authoritative_overrides(
614+
{},
615+
recipe_ingredients,
616+
tmp_path,
617+
)
618+
619+
assert "source_dir" not in result
620+
621+
def test_apply_config_authoritative_overrides_only_mutates_enforcement_keys(self, tmp_path):
622+
"""For a recipe that declares ALL CONFIG_AUTHORITY_KEYS as authority='config',
623+
the function must only mutate keys in SERVER_AUTHORITATIVE_INGREDIENTS |
624+
BACKEND_CAPABILITY_INGREDIENTS. source_dir is caller-sovereign and must not
625+
appear in the changed-key set."""
626+
from types import SimpleNamespace
627+
from unittest.mock import patch
628+
629+
from autoskillit.config import apply_config_authoritative_overrides
630+
from autoskillit.config.ingredient_defaults import (
631+
BACKEND_CAPABILITY_INGREDIENTS,
632+
SERVER_AUTHORITATIVE_INGREDIENTS,
633+
)
634+
from autoskillit.core import CONFIG_AUTHORITY_KEYS
635+
636+
recipe_ingredients = {
637+
key: SimpleNamespace(authority="config") for key in CONFIG_AUTHORITY_KEYS
638+
}
639+
resolved_values = {key: f"resolved-{key}" for key in CONFIG_AUTHORITY_KEYS}
640+
capability_values = {key: f"cap-{key}" for key in BACKEND_CAPABILITY_INGREDIENTS}
641+
642+
with patch(
643+
"autoskillit.config.ingredient_defaults.resolve_ingredient_defaults",
644+
return_value=resolved_values,
645+
):
646+
result = apply_config_authoritative_overrides(
647+
{},
648+
recipe_ingredients,
649+
tmp_path,
650+
capability_overrides=capability_values,
651+
)
652+
653+
changed_keys = set(result.keys())
654+
assert changed_keys <= (
655+
SERVER_AUTHORITATIVE_INGREDIENTS | BACKEND_CAPABILITY_INGREDIENTS
656+
), (
657+
f"apply_config_authoritative_overrides mutated caller-sovereign keys: "
658+
f"{changed_keys - (SERVER_AUTHORITATIVE_INGREDIENTS | BACKEND_CAPABILITY_INGREDIENTS)}"
659+
)
660+
540661

541662
def test_capability_overrides_dict_covers_registry_keys():
542663
"""Structural test: every key in BACKEND_CAPABILITY_INGREDIENTS must appear in the

0 commit comments

Comments
 (0)