Skip to content

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, called from_yaml(link), asserted errno == 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 Result instance. PASS.
  • AC-13 defaults: "When you don't say precedence: in the YAML, the answer is 50, not 0." ::test_minimal_yaml_pins_documented_defaults asserts m.precedence == 50 by 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

  • SizeCapExceeded variant name collides with codegenie.errors.SizeCapExceeded marker — aliased the import: from codegenie.errors import SizeCapExceeded as _SafeYamlSizeCap. Same trick for SymlinkRefusedError. The variant-name choice is fixed by AC-6 (story-prescribed); the alias is the surgical workaround.
  • DepthCapExceeded routes to MalformedYaml (not enumerated explicitly in AC-9). The story names four arms but does not say what to do with DepthCapExceeded; routing it to MalformedYaml (alongside MalformedYAMLError) 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 — chose mode="after" rather than mode="before" because Pydantic's tuple-of-str validation is the natural pre-step; the after-validator gets a typed tuple, lifts each element, raises ValueError on failure (Pydantic wraps into ValidationError, from_yaml translates to SchemaViolation). Mirrors the canonical TCCM loader's "validate first, then refine" discipline.
  • _render_field_errors extracted as file-local helper — story-prescribed (Refactor §). Renders ve.errors()[*]['loc'] as dotted strings; pinned against Pydantic v2 minor upgrades via comment.
  • _restat_or_zero(path) -> int helper — story-prescribed (translation table §SizeCapExceeded). Failed re-stat returns 0 rather than masking the original size-cap refusal with an IoError — the caller still gets the size-cap variant.
  • NO PluginManifestLoader class — 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 modulesafe_yaml is the chokepoint; manifest.py is a consumer, not a new entrypoint (story Notes §1).

Anti-pattern smells walked (none introduced)

  • No Any in src/codegenie/plugins/manifest.py (fully typed; Mapping[str, JSONValue] from safe_yaml.load is dict-cast at the validator boundary — the only type-narrowing in the loader).
  • No raw str for PluginId / PrimitiveName / ProbeId in production code; lifts happen via parse_plugin_id field validators.
  • No if/elif ladders over closed sets — the translation table is try/except arms 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 BaseModel directly.
  • __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 of scope: ManifestScope (load-time raw form). Logged in _validation/S2-02-plugin-manifest-pydantic.md per the validator's pass; not in scope for this implementation.
  • Phase-arch §Data model line 756 still shows precedence: int = 0; canonical default is 50. Same arch-amendment follow-up.
  • The pre-commit/lint-imports canary tests fail under bare pytest when .venv/bin is not on PATH; pass under make check and CI. Pre-existing flake, not introduced by this story.