|
47 | 47 | def _setup_project_recipe(tmp_path: Path, name: str, content: str) -> Path: |
48 | 48 | """Write a recipe YAML to tmp_path/.autoskillit/recipes/<name>.yaml.""" |
49 | 49 | recipes_dir = tmp_path / ".autoskillit" / "recipes" |
50 | | - recipes_dir.mkdir(parents=True) |
| 50 | + recipes_dir.mkdir(parents=True, exist_ok=True) |
51 | 51 | recipe_path = recipes_dir / f"{name}.yaml" |
52 | 52 | recipe_path.write_text(content) |
53 | 53 | return recipe_path |
@@ -1262,3 +1262,242 @@ def test_validate_from_path_codex_backend_valid_after_pruning(tmp_path: Path) -> |
1262 | 1262 | if s.get("severity") == Severity.ERROR |
1263 | 1263 | ) |
1264 | 1264 | ) |
| 1265 | + |
| 1266 | + |
| 1267 | +# --------------------------------------------------------------------------- |
| 1268 | +# load_and_validate fail-closed tests (regression for steps-less YAML bug) |
| 1269 | +# --------------------------------------------------------------------------- |
| 1270 | + |
| 1271 | + |
| 1272 | +_RECIPE_NO_STEPS = """\ |
| 1273 | +name: test-no-steps |
| 1274 | +description: A test recipe with no steps key |
| 1275 | +autoskillit_version: "0.3.0" |
| 1276 | +kitchen_rules: |
| 1277 | + - "Test rule" |
| 1278 | +""" |
| 1279 | + |
| 1280 | +_RECIPE_EMPTY_STEPS = """\ |
| 1281 | +name: test-empty-steps |
| 1282 | +description: A test recipe with empty steps |
| 1283 | +autoskillit_version: "0.3.0" |
| 1284 | +kitchen_rules: |
| 1285 | + - "Test rule" |
| 1286 | +steps: {} |
| 1287 | +""" |
| 1288 | + |
| 1289 | +_RECIPE_NON_DICT_STEPS = """\ |
| 1290 | +name: test-list-steps |
| 1291 | +description: A test recipe with steps as a list |
| 1292 | +autoskillit_version: "0.3.0" |
| 1293 | +kitchen_rules: |
| 1294 | + - "Test rule" |
| 1295 | +steps: |
| 1296 | + - foo |
| 1297 | + - bar |
| 1298 | +""" |
| 1299 | + |
| 1300 | +_RECIPE_NON_DICT_ROOT = """\ |
| 1301 | +- item1 |
| 1302 | +- item2 |
| 1303 | +""" |
| 1304 | + |
| 1305 | +_RECIPE_CAMPAIGN_NO_STEPS = """\ |
| 1306 | +name: test-campaign-no-steps |
| 1307 | +description: A campaign recipe with dispatches and no steps |
| 1308 | +autoskillit_version: "0.3.0" |
| 1309 | +kind: campaign |
| 1310 | +kitchen_rules: |
| 1311 | + - "Campaign rule" |
| 1312 | +dispatches: |
| 1313 | + - name: dispatch1 |
| 1314 | + recipe: some-recipe |
| 1315 | +""" |
| 1316 | + |
| 1317 | + |
| 1318 | +# 1a: steps-less YAML returns valid=False |
| 1319 | +def test_load_and_validate_steps_less_yaml_returns_invalid(tmp_path: Path) -> None: |
| 1320 | + """A YAML recipe without a 'steps' key must produce valid=False.""" |
| 1321 | + import autoskillit.recipe._api_cache as cache_mod |
| 1322 | + |
| 1323 | + cache_mod._LOAD_CACHE.clear() |
| 1324 | + _setup_project_recipe(tmp_path, "test-no-steps", _RECIPE_NO_STEPS) |
| 1325 | + |
| 1326 | + from autoskillit.recipe._api import load_and_validate |
| 1327 | + |
| 1328 | + result = load_and_validate("test-no-steps", project_dir=tmp_path) |
| 1329 | + |
| 1330 | + assert result["valid"] is False, ( |
| 1331 | + f"Steps-less YAML must return valid=False; got valid={result.get('valid')}. " |
| 1332 | + f"Errors: {result.get('errors', [])}" |
| 1333 | + ) |
| 1334 | + assert len(result["errors"]) > 0, "Expected at least one error for steps-less YAML" |
| 1335 | + assert any("step" in e.lower() for e in result["errors"]), ( |
| 1336 | + f"Expected an error mentioning steps; got: {result['errors']}" |
| 1337 | + ) |
| 1338 | + assert any(s.get("severity") == Severity.ERROR for s in result.get("suggestions", [])), ( |
| 1339 | + "Expected at least one ERROR severity suggestion" |
| 1340 | + ) |
| 1341 | + |
| 1342 | + |
| 1343 | +# 1b: cached result also returns valid=False |
| 1344 | +def test_load_and_validate_steps_less_yaml_not_cached_as_valid(tmp_path: Path) -> None: |
| 1345 | + """After calling load_and_validate with steps-less YAML, the cache must not serve valid=True.""" # noqa: E501 |
| 1346 | + import autoskillit.recipe._api_cache as cache_mod |
| 1347 | + |
| 1348 | + cache_mod._LOAD_CACHE.clear() |
| 1349 | + _setup_project_recipe(tmp_path, "test-no-steps", _RECIPE_NO_STEPS) |
| 1350 | + |
| 1351 | + from autoskillit.recipe._api import load_and_validate |
| 1352 | + |
| 1353 | + result1 = load_and_validate("test-no-steps", project_dir=tmp_path) |
| 1354 | + result2 = load_and_validate("test-no-steps", project_dir=tmp_path) |
| 1355 | + |
| 1356 | + assert result1["valid"] is False |
| 1357 | + assert result2["valid"] is False, ( |
| 1358 | + f"Cached result must also be valid=False; got valid={result2.get('valid')}" |
| 1359 | + ) |
| 1360 | + |
| 1361 | + |
| 1362 | +# 1c: compute_recipe_validity is always the source of valid |
| 1363 | +def test_load_and_validate_always_invokes_compute_recipe_validity( |
| 1364 | + tmp_path: Path, monkeypatch: pytest.MonkeyPatch |
| 1365 | +) -> None: |
| 1366 | + """Monkey-patch compute_recipe_validity with a sentinel and verify it flows through |
| 1367 | + for BOTH with-steps and without-steps recipes. This structurally guarantees that |
| 1368 | + no code path through load_and_validate can return a valid value that didn't come |
| 1369 | + from compute_recipe_validity. |
| 1370 | + """ |
| 1371 | + import autoskillit.recipe._api as api_mod |
| 1372 | + import autoskillit.recipe._api_cache as cache_mod |
| 1373 | + |
| 1374 | + cache_mod._LOAD_CACHE.clear() |
| 1375 | + |
| 1376 | + _setup_project_recipe(tmp_path, "test-no-steps", _RECIPE_NO_STEPS) |
| 1377 | + _setup_project_recipe(tmp_path, "test-recipe-with-rules", _RECIPE_WITH_RULES) |
| 1378 | + |
| 1379 | + sentinel_valid = object() |
| 1380 | + monkeypatch.setattr(api_mod, "compute_recipe_validity", lambda *a, **kw: sentinel_valid) |
| 1381 | + |
| 1382 | + from autoskillit.recipe._api import load_and_validate |
| 1383 | + |
| 1384 | + result_no_steps = load_and_validate("test-no-steps", project_dir=tmp_path) |
| 1385 | + assert result_no_steps["valid"] is sentinel_valid, ( |
| 1386 | + f"Steps-less recipe must get valid from compute_recipe_validity sentinel; " |
| 1387 | + f"got valid={result_no_steps.get('valid')!r}" |
| 1388 | + ) |
| 1389 | + |
| 1390 | + cache_mod._LOAD_CACHE.clear() |
| 1391 | + result_with_steps = load_and_validate("test-recipe-with-rules", project_dir=tmp_path) |
| 1392 | + assert result_with_steps["valid"] is sentinel_valid, ( |
| 1393 | + f"With-steps recipe must get valid from compute_recipe_validity sentinel; " |
| 1394 | + f"got valid={result_with_steps.get('valid')!r}" |
| 1395 | + ) |
| 1396 | + |
| 1397 | + |
| 1398 | +# 1d: empty-steps recipe returns valid=False |
| 1399 | +def test_load_and_validate_empty_steps_returns_invalid(tmp_path: Path) -> None: |
| 1400 | + """A YAML with steps: {} (present but empty) must produce valid=False.""" |
| 1401 | + import autoskillit.recipe._api_cache as cache_mod |
| 1402 | + |
| 1403 | + cache_mod._LOAD_CACHE.clear() |
| 1404 | + _setup_project_recipe(tmp_path, "test-empty-steps", _RECIPE_EMPTY_STEPS) |
| 1405 | + |
| 1406 | + from autoskillit.recipe._api import load_and_validate |
| 1407 | + |
| 1408 | + result = load_and_validate("test-empty-steps", project_dir=tmp_path) |
| 1409 | + |
| 1410 | + assert result["valid"] is False, ( |
| 1411 | + f"Empty-steps YAML must return valid=False; got valid={result.get('valid')}. " |
| 1412 | + f"Errors: {result.get('errors', [])}" |
| 1413 | + ) |
| 1414 | + assert any("step" in e.lower() for e in result["errors"]), ( |
| 1415 | + f"Expected error about steps; got: {result['errors']}" |
| 1416 | + ) |
| 1417 | + |
| 1418 | + |
| 1419 | +# 1e: non-dict YAML returns valid=False |
| 1420 | +def test_load_and_validate_non_dict_root_returns_invalid(tmp_path: Path) -> None: |
| 1421 | + """A YAML that parses to a list (non-dict root) must produce valid=False. |
| 1422 | +
|
| 1423 | + Non-dict root YAML cannot be discovered by list_recipes (which catches the |
| 1424 | + ValueError and skips the file), so we construct a RecipeInfo directly. |
| 1425 | + """ |
| 1426 | + import autoskillit.recipe._api_cache as cache_mod |
| 1427 | + from autoskillit.core.types import RecipeSource |
| 1428 | + from autoskillit.recipe._api import load_and_validate |
| 1429 | + from autoskillit.recipe.schema import RecipeInfo |
| 1430 | + |
| 1431 | + cache_mod._LOAD_CACHE.clear() |
| 1432 | + recipe_path = _setup_project_recipe(tmp_path, "test-list-root", _RECIPE_NON_DICT_ROOT) |
| 1433 | + |
| 1434 | + info = RecipeInfo( |
| 1435 | + name="test-list-root", |
| 1436 | + description="", |
| 1437 | + source=RecipeSource.PROJECT, |
| 1438 | + path=recipe_path, |
| 1439 | + content=_RECIPE_NON_DICT_ROOT, |
| 1440 | + ) |
| 1441 | + |
| 1442 | + result = load_and_validate("test-list-root", project_dir=tmp_path, recipe_info=info) |
| 1443 | + |
| 1444 | + assert result["valid"] is False, ( |
| 1445 | + f"Non-dict root YAML must return valid=False; got valid={result.get('valid')}" |
| 1446 | + ) |
| 1447 | + |
| 1448 | + |
| 1449 | +# 1f: non-dict steps value returns valid=False |
| 1450 | +def test_load_and_validate_non_dict_steps_value_returns_invalid(tmp_path: Path) -> None: |
| 1451 | + """A YAML with steps: [foo, bar] (list instead of mapping) must produce valid=False |
| 1452 | + with a clear error, not an uncaught AttributeError. |
| 1453 | +
|
| 1454 | + Non-dict steps YAML cannot be discovered by list_recipes (the new ValueError |
| 1455 | + guard in _parse_recipe causes _collect_recipes to skip it), so we construct |
| 1456 | + a RecipeInfo directly. |
| 1457 | + """ |
| 1458 | + import autoskillit.recipe._api_cache as cache_mod |
| 1459 | + from autoskillit.core.types import RecipeSource |
| 1460 | + from autoskillit.recipe._api import load_and_validate |
| 1461 | + from autoskillit.recipe.schema import RecipeInfo |
| 1462 | + |
| 1463 | + cache_mod._LOAD_CACHE.clear() |
| 1464 | + recipe_path = _setup_project_recipe(tmp_path, "test-list-steps", _RECIPE_NON_DICT_STEPS) |
| 1465 | + |
| 1466 | + info = RecipeInfo( |
| 1467 | + name="test-list-steps", |
| 1468 | + description="A test recipe with steps as a list", |
| 1469 | + source=RecipeSource.PROJECT, |
| 1470 | + path=recipe_path, |
| 1471 | + content=_RECIPE_NON_DICT_STEPS, |
| 1472 | + ) |
| 1473 | + |
| 1474 | + result = load_and_validate("test-list-steps", project_dir=tmp_path, recipe_info=info) |
| 1475 | + |
| 1476 | + assert result["valid"] is False, ( |
| 1477 | + f"Non-dict steps value must return valid=False; got valid={result.get('valid')}. " |
| 1478 | + f"Suggestions: {result.get('suggestions', [])}" |
| 1479 | + ) |
| 1480 | + |
| 1481 | + |
| 1482 | +# 1g: campaign recipe with no steps does not get "must have at least one step" error |
| 1483 | +def test_load_and_validate_campaign_no_steps_returns_valid(tmp_path: Path) -> None: |
| 1484 | + """A campaign recipe (kind=campaign) with dispatches and no steps must NOT |
| 1485 | + receive the structural "must have at least one step" error. |
| 1486 | +
|
| 1487 | + Campaign recipes take the CAMPAIGN early-return path in validate_recipe_structure |
| 1488 | + and are validated via their dispatches, not steps. This guards against the |
| 1489 | + removal of the ``"steps" in data`` guard breaking campaign recipes. |
| 1490 | + """ |
| 1491 | + import autoskillit.recipe._api_cache as cache_mod |
| 1492 | + |
| 1493 | + cache_mod._LOAD_CACHE.clear() |
| 1494 | + _setup_project_recipe(tmp_path, "test-campaign-no-steps", _RECIPE_CAMPAIGN_NO_STEPS) |
| 1495 | + |
| 1496 | + from autoskillit.recipe._api import load_and_validate |
| 1497 | + |
| 1498 | + result = load_and_validate("test-campaign-no-steps", project_dir=tmp_path) |
| 1499 | + |
| 1500 | + step_errors = [e for e in result.get("errors", []) if "step" in e.lower()] |
| 1501 | + assert not step_errors, ( |
| 1502 | + f"Campaign recipe should not get step-related structural errors; got: {step_errors}" |
| 1503 | + ) |
0 commit comments