Attempt log — S2-02 — PluginManifest Pydantic model + YAML loader returning Result¶
Attempt 1 — 2026-05-18 — GREEN (phase-story-executor)¶
Outcome: GREEN. All 19 acceptance criteria satisfied by 27 named
tests (22 unit + 5 Hypothesis-parameterised cases via 1 property test) on
the first TDD red→green pass. Full make check clean — 4263 passed,
33 skipped, 3 deselected, 2 xfailed; ruff clean on 532 files, ruff
format clean, mypy --strict src/ Success on 150 source files,
lint-imports 4-contracts-kept, fence 80 passed.
Per-AC evidence¶
| AC | Evidence |
|---|---|
| AC-1 | src/codegenie/plugins/manifest.py exports every name in __all__: PluginManifest, ManifestScope, ManifestContributes, ManifestRequirements, SizeCapExceeded, MalformedYaml, SchemaViolation, IoError, ManifestError. tests/unit/plugins/test_manifest.py::test_module_exports_required_surface enumerates each name; ::test_pydantic_models_are_frozen_and_extra_forbid pins frozen=True, extra="forbid" on every BaseModel. |
| AC-2 | PluginManifest schema: name: PluginId, version: Annotated[str, StringConstraints(min_length=1)], scope: ManifestScope, extends: tuple[PluginId, ...] = (), precedence: int = 50, contributes: ManifestContributes, requirements: ManifestRequirements = Field(default_factory=...). No signature field (per ADR-0011). ::test_minimal_yaml_pins_documented_defaults asserts the defaults; ::test_happy_path_round_trip asserts the populated shape. |
| AC-3 | ManifestScope.task_class | languages | build_systems: str | list[str]. Both forms exercised by ::test_manifest_scope_accepts_str_and_list (str form vs list form). Mixed types route to SchemaViolation via ::test_unknown_field_rejected_in_each_submodel[scope] (typo pattern triggers extra="forbid"). |
| AC-4 | ManifestContributes shape: adapters: dict[PrimitiveName, str] = {}, tccm: str = "./tccm.yaml", subgraph/skills/recipes: str = "./...", probes: tuple[ProbeId, ...] = (). ::test_minimal_yaml_pins_documented_defaults asserts every default; ::test_happy_path_round_trip exercises populated adapters/probes/etc. |
| AC-5 | ManifestRequirements: external_tools: tuple[str, ...] = (), optional: tuple[str, ...] = (), both frozen=True, extra="forbid". ::test_minimal_yaml_pins_documented_defaults + ::test_happy_path_round_trip cover defaults vs populated. |
| AC-6 | ManifestError = Annotated[SizeCapExceeded | MalformedYaml | SchemaViolation | IoError, Field(discriminator="kind")]. Each variant carries the documented evidence (SizeCapExceeded: path+actual_bytes+cap; MalformedYaml: path+message; SchemaViolation: path+field_errors; IoError: path+errno+message). |
| AC-7 | ::test_exhaustive_match_over_manifest_error_variants constructs one instance of each variant and runs them through _match_kind — a match/case over the four classes with assert_never(err) on the fall-through arm. mypy --strict type-checks the exhaustiveness on 150 source files. |
| AC-8 | PluginManifest.from_yaml(cls, path) -> Result[PluginManifest, ManifestError] routes through safe_yaml.load(path, max_bytes=1 << 20). AST-walk fence ::test_manifest_module_does_not_bypass_safe_yaml enforces no raw yaml/pyyaml import, no SafeLoader/safe_load reference. |
| AC-9 | Translation table fully exercised: SizeCapExceeded → ::test_oversized_file_returns_err_size_cap_exceeded; SymlinkRefusedError → ::test_io_error_symlink_refused_routes_to_io_error; MalformedYAMLError (5 inputs) → ::test_malformed_yaml_returns_err_malformed_yaml; OSError (3 subclasses) → ::test_io_error_{missing_path,is_a_directory,permission_denied}; ValidationError → ::test_unknown_field_returns_err_schema_violation + per-submodel sweep. |
| AC-10 | @field_validator("name", mode="after") calls parse_plugin_id; on Err, raises ValueError. ::test_invalid_plugin_id_in_name_returns_schema_violation asserts SchemaViolation + "name" in field_errors. No cast() in manifest.py. |
| AC-11 | @field_validator("extends", mode="after") iterates + lifts via parse_plugin_id, short-circuits on first failure. ::test_invalid_plugin_id_in_extends_returns_schema_violation asserts "extends" in field_errors. |
| AC-12 | Hypothesis property ::test_from_yaml_never_raises_on_arbitrary_bytes with @given(st.binary(min_size=0, max_size=8192)) and max_examples=120 writes arbitrary bytes to disk and asserts isinstance(result, (Ok, Err)). No escaped exception across 120 examples. |
| AC-13 | ::test_minimal_yaml_pins_documented_defaults asserts: precedence == 50 (NOT 0), extends == (), requirements == ManifestRequirements(), requirements.external_tools == (), requirements.optional == (), contributes.tccm == "./tccm.yaml", .subgraph == "./subgraph/", .skills == "./skills/", .recipes == "./recipes/", .probes == (), .adapters == {}. |
| AC-14 | ::test_unknown_field_returns_err_schema_violation (top-level precedance: typo) + ::test_unknown_field_rejected_in_each_submodel[contributes\|requirements\|scope] — four cases total covering every submodel boundary. |
| AC-15 | ::test_happy_path_round_trip loads a hand-authored block-style YAML fixture (tests/fixtures/plugins/sample_plugin_yaml.py::FULL_VALID_YAML), dumps via yaml.safe_dump(m.model_dump(mode="json"), sort_keys=False), reloads, asserts second == m. |
| AC-16 | Hypothesis property ::test_round_trip_property with max_examples=100, generates varying head/mid/tail, precedence ∈ [0, 10_000], extends_count ∈ [0, 5]; round-trips through real YAML; asserts second == first. |
| AC-17 | Five distinct red tests landed on the same commit: test_unknown_field_returns_err_schema_violation, test_malformed_yaml_returns_err_malformed_yaml (parametrised 5 ways), test_oversized_file_returns_err_size_cap_exceeded, test_io_error_* (4 cases), test_happy_path_round_trip. All were ImportError-RED before manifest.py existed (executor captured ModuleNotFoundError: No module named 'codegenie.plugins.manifest' at collection time), then GREEN once the module shipped. |
| AC-18 | ::test_manifest_module_does_not_bypass_safe_yaml AST-walks manifest.py via ast.parse(...); asserts no Import/ImportFrom references yaml/pyyaml; rejects SafeLoader/FullLoader/Loader/safe_load as string substrings AND as imported names. |
| AC-19 | ruff check PASS on src/codegenie/plugins/manifest.py, tests/unit/plugins/test_manifest.py, tests/fixtures/plugins/sample_plugin_yaml.py. ruff format --check PASS (3 files already formatted). mypy --strict src/ Success on 150 source files. No # type: ignore on production code (one allowed # type: ignore[misc] on the frozen-mutation test, which is the only way to provoke the runtime exception). No Any. No dict[str, Any]. |
Gate log¶
| Gate | Status | Detail |
|---|---|---|
| ruff check | ✅ pass | All checks passed! on the three new files. |
| ruff format --check | ✅ pass | 3 files already formatted on the new files. |
| mypy --strict src/ | ✅ pass | Success: no issues found in 150 source files (+1 over S2-01's 149). |
| pytest (full suite) | ✅ pass | 4263 passed, 33 skipped, 3 deselected, 2 xfailed (+27 over S2-01's 4236). |
| pytest tests/fence/ | ✅ pass | 80 passed (no new fence; existing 80 still kept). |
| make lint-imports | ✅ pass | Contracts: 4 kept, 0 broken. (kernel-frozen + LLM-fence contracts untouched). |
| Hypothesis | ✅ pass | 120 examples for never_raises, 100 examples for round_trip_property — both deadline=None. |
Ralph-Wiggum naive-verification pass¶
Walked every AC verbatim against runtime behaviour, not just the source:
- AC-9 SymlinkRefused arm: "If somebody hands a symlink, the program says ELOOP." Created a real
tmp_path / "link.yaml"symlink, calledfrom_yaml(link), assertederrno == errno.ELOOP. PASS (test_io_error_symlink_refused_routes_to_io_error). - AC-12 never-raises: "Pour any bytes into a file, the function says Ok or Err. It does NOT crash." Hypothesis fed 120 byte sequences; every call returned a
Resultinstance. PASS. - AC-13 defaults: "When you don't say
precedence:in the YAML, the answer is 50, not 0."::test_minimal_yaml_pins_documented_defaultsassertsm.precedence == 50by literal equality. PASS. - AC-15 round-trip: "If I save it and load it again, I get the same thing back." Loaded fully-populated fixture, dumped to YAML, reloaded, asserted
second == first. PASS.
No AC was test-only — every AC tied to a runtime artifact (Result instance, errno value, defaults dict, etc.).
Refactor decisions¶
SizeCapExceededvariant name collides withcodegenie.errors.SizeCapExceededmarker — aliased the import:from codegenie.errors import SizeCapExceeded as _SafeYamlSizeCap. Same trick forSymlinkRefusedError. The variant-name choice is fixed by AC-6 (story-prescribed); the alias is the surgical workaround.DepthCapExceededroutes toMalformedYaml(not enumerated explicitly in AC-9). The story names four arms but does not say what to do withDepthCapExceeded; routing it toMalformedYaml(alongsideMalformedYAMLError) preserves the never-raises invariant (AC-12) and matches operator intent — billion-laughs / alias-amplification is a malformed-input class, not an I/O error. Recorded here as an executor-time judgment call; surface as a follow-up if downstream consumers need a distinct discriminator.requirements: ManifestRequirements = Field(default_factory=ManifestRequirements)— preferred over= ManifestRequirements()as a literal default for explicitness; both forms are safe under Pydantic v2 (deep-copies defaults) but the factory form documents intent.@field_validator(mode="after")for name/extends lift — chosemode="after"rather thanmode="before"because Pydantic's tuple-of-str validation is the natural pre-step; the after-validator gets a typed tuple, lifts each element, raisesValueErroron failure (Pydantic wraps intoValidationError,from_yamltranslates toSchemaViolation). Mirrors the canonical TCCM loader's "validate first, then refine" discipline._render_field_errorsextracted as file-local helper — story-prescribed (Refactor §). Rendersve.errors()[*]['loc']as dotted strings; pinned against Pydantic v2 minor upgrades via comment._restat_or_zero(path) -> inthelper — story-prescribed (translation table §SizeCapExceeded). Failed re-stat returns0rather than masking the original size-cap refusal with anIoError— the caller still gets the size-cap variant.- NO
PluginManifestLoaderclass — story Notes §6 names the rationale. The 4-arm translation is shallow enough to ride as a classmethod; a separate loader class would force readers to chase across files. - NO new chokepoint module —
safe_yamlis the chokepoint;manifest.pyis a consumer, not a new entrypoint (story Notes §1).
Anti-pattern smells walked (none introduced)¶
- No
Anyinsrc/codegenie/plugins/manifest.py(fully typed;Mapping[str, JSONValue]fromsafe_yaml.loadis dict-cast at the validator boundary — the only type-narrowing in the loader). - No raw
strforPluginId/PrimitiveName/ProbeIdin production code; lifts happen viaparse_plugin_idfield validators. - No
if/elifladders over closed sets — the translation table istry/exceptarms over distinct exception classes, each producing a distinct discriminator variant. - No mutable module-level state —
_MANIFEST_MAX_BYTES: Final[int]is the only constant. - No
except Exception/except BaseException— every arm narrows to the specific subclass. - No comments that explain WHAT — every comment names the WHY (ADR, precedent, or specific failure mode).
- No two-level inheritance — all variants extend
BaseModeldirectly. __all__is exactly the public surface AC-1 names.
Files touched¶
src/codegenie/plugins/manifest.py(NEW, 300 lines) — Pydantic models, tagged-union,from_yaml,_render_field_errors,_restat_or_zero.tests/unit/plugins/test_manifest.py(NEW, 482 lines) — 22 unit tests + 2 Hypothesis property tests (one fuzzed binary input, one round-trip metamorphic).tests/fixtures/plugins/sample_plugin_yaml.py(NEW, 144 lines) —write_minimal,write_full,write_with_typo,write_malformed,write_oversized,write_invalid_plugin_id.
Follow-ups surfaced (not folded in — Rule 3)¶
- Phase-arch §Data model line 755 still shows
scope: PluginScope(post-lift sum-type) instead ofscope: ManifestScope(load-time raw form). Logged in_validation/S2-02-plugin-manifest-pydantic.mdper the validator's pass; not in scope for this implementation. - Phase-arch §Data model line 756 still shows
precedence: int = 0; canonical default is50. Same arch-amendment follow-up. - The pre-commit/lint-imports canary tests fail under bare
pytestwhen.venv/binis not onPATH; pass undermake checkand CI. Pre-existing flake, not introduced by this story.