fix(packages): implement PackageRecipe.to_dict() to resolve index syn…

[dkonlycomputer]

…c AttributeError

Summary

Type of Change

• eat — New feature
• ix — Bug fix
• docs — Documentation only
• style — Formatting, no code change
• [ ]
  efactor — Code restructuring without behavior change
• est — Add or fix tests
• �uild — Build system or dependency changes
• ci — CI/CD pipeline changes
• perf — Performance improvement

Changes

Testing

• Unit tests pass (ctest --test-dir build --output-on-failure)
• Integration tests pass
• Manual testing performed
• New tests added for new functionality

Pre-Submission Checklist

• Code compiles without warnings (-Wall -Wextra -Werror for C)
• All existing tests pass
• New tests added for new functionality
• Documentation updated if API changed
• Commit messages follow (): convention
• Branch is rebased on latest master

Related Issues

Screenshots / Logs

Additional Notes

[srpatcha]

Review — ebuild#119 "fix(packages): implement PackageRecipe.to_dict() to resolve index sync AttributeError"

head: aec31c3 author: dkonlycomputer ci: none reported

Verdict: Fixes a real AttributeError — ebuild/packages/index_sync.py:354 calls recipe.to_dict() on a PackageRecipe that had no such method, so every remote-index sync that reached a recipe with a URL crashed. The fix works (verified by round-trip below), but it writes cache files with different key names than every shipped recipe, and it arrives with no test, no CI and an untouched PR template.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	ebuild/packages/recipe.py:92-95 → ebuild/packages/index_sync.py:354	asdict() emits the dataclass field names, so the YAML written into the recipe cache starts name: zlib / build_system: cmake, while all 10 shipped recipes and the documented format use package: / build: (recipes/cjson.yaml:1,8). I ran the round trip: it does load back correctly, because parse_recipe() carries fallbacks at recipe.py:141 (raw.get("package", raw.get("name", ""))) and recipe.py:145 (raw.get("build", raw.get("build_system", "cmake"))). So this is not broken today — it is load-bearing on two fallbacks that exist for legacy input, now silently promoted to the format this code writes. Remove either fallback and the whole cache becomes unloadable, with no test to catch it. The dependencies/depends pair is also asymmetric: the writer emits dependencies, the reader prefers it, but nothing pins that.	Serialise to the canonical keys explicitly rather than leaning on asdict(): return {"package": self.name, "version": self.version, "description": self.description, "license": self.license, "url": self.url, "checksum": self.checksum, "build": self.build_system, "dependencies": list(self.dependencies), "patches": list(self.patches), "configure_args": list(self.configure_args), "build_args": list(self.build_args), "install_args": list(self.install_args)} — then a cached recipe is byte-comparable in shape to a shipped one, and the legacy fallbacks stay legacy.
2	Medium	tests/ebuild/test_package_recipe.py	New public method, no test. The file already has six tests covering exactly this parse surface (test_parse_recipe_public_name_preserves_legacy_alias, test_depends_alias_accepts_a_list, …), so the missing case is conspicuous: nothing pins that parse_recipe(x).to_dict() reloads to an equal recipe, which is the property index_sync.py depends on. The PR checklist's "New tests added for new functionality" is unchecked, so this is acknowledged rather than overlooked.	Add to tests/ebuild/test_package_recipe.py: def test_to_dict_round_trips_through_yaml(): build a recipe via load_recipe_from_string, yaml.safe_dump(r.to_dict()), reload, assert r2 == r. Two lines more asserts the emitted keys are the canonical ones, which locks in finding 1's fix.
3	Medium	PR body	The body is the unmodified template: Summary empty, Changes empty (- / - ), every Type-of-Change and Pre-Submission box unchecked, all <!-- ... --> placeholders intact. Combined with finding 4 there is no evidence of any kind that this was run — not a false claim, but no claim either. For a change to the code path that writes the on-disk recipe cache, "Unit tests pass" and "All existing tests pass" being unchecked is the material fact.	Fill in Summary/Changes, and state what was actually run — at minimum python3 -m pytest tests/ebuild/test_package_recipe.py -q and one real ebuild search/index-sync invocation showing the AttributeError is gone.
4	Medium	PR CI	No checks ran. gh pr checks 119 reports "no checks reported on the 'fix/package-recipe-to-dict' branch"; the bundle's checks.txt is empty; the PR is BLOCKED. Nothing has been linted or tested by CI.	Re-trigger the workflow on this branch.
5	Low	ebuild/packages/recipe.py:94	from dataclasses import asdict is a function-local import, while line 12 of the same file already does from dataclasses import dataclass, field at module scope, and the sibling ebuild/packages/repository.py:14 imports asdict at module scope. No circular-import reason exists here.	Add asdict to the module-level import on line 12 and drop the local one.

Not a finding, checked and clear: ruff check with this repo's own configuration (pyproject.toml:41-51, select = ["E","F","W"]) passes on the changed file — the three blank lines at recipe.py:96-98 are not flagged. VALID_BUILD_SYSTEMS and _SHA256_RE are unannotated class attributes, so asdict() correctly omits them; all twelve emitted values are str or List[str] and yaml.safe_dump handles them without a representer error.

Architecture conformance

Conforms. Master design §9 places eBuild as the developer control plane and §9.2 requires "reproducible lockfiles/manifests for production builds" and "machine-readable output mode" — serialising a recipe back to its on-disk form serves both. §10 ("Component and Manifest System") and §23.2's "Package format — versioned .epkg/.eapp metadata schema" are the reason finding 1 matters: the manifest schema is a compatibility contract, so the writer and the shipped files should not disagree about field names. §21 tier placement is correct — ebuild is Tier 1 Foundation and package-recipe serialisation belongs in ebuild/packages/, not in a consumer. No dependency direction changes; recipe.py imports only stdlib and yaml, so nothing points up a tier. §5.1 unaffected — eBuild is not a runtime dependency and this change does not make it one.

Proposed changes

1. Replace asdict(self) with the explicit canonical-key mapping in finding 1, and hoist the asdict import out (or drop it entirely once the mapping is explicit).
2. Add the round-trip test in finding 2, asserting both r2 == r and set(d) >= {"package", "build"}.
3. Run python3 -m pytest tests/ -q and ruff check ., and paste both into the PR body along with a filled-in Summary.

Order matters only in that the test should be written against the canonical keys, so do 1 before 2 or write 2 to assert the intended shape.

Not checked

• The actual defect was not reproduced. I confirmed by reading that index_sync.py:354 calls .to_dict() and that no such method exists on PackageRecipe before this diff; I did not run an index sync against a live or fixture remote index to observe the AttributeError.
• The repository's test suite was not run. ebuild has a dirty working tree (4 files) and this pipeline leaves such repos untouched, so pytest tests/ was NOT RUN. The round-trip check and the ruff run above were performed on a copy of the head revision of ebuild/packages/recipe.py extracted to a temporary directory, in isolation from the rest of the package — that verifies the function and the key names, not integration.
• Whether any consumer outside this repository reads the cached recipe files and expects package:/build:. Only ebuild was searched.
• The pruning logic at index_sync.py:361-375 that runs immediately after these writes, and whether the .yml/.yaml stem bookkeeping is correct — outside this diff and not examined.
• Separate defect, not attributable to this PR: .github/PULL_REQUEST_TEMPLATE.md contains literal control characters where escape sequences were intended — form feed before eat, form feed before ix, CR before efactor, tab before est, backspace before uild (i.e. \feat, \fix, \refactor, \test, \build were interpreted). That is why this PR's body renders "eat — New feature" and "ix — Bug fix". Every contributor to this repo hits it. Worth a one-line fix to the template, but it is not this author's to make here.

---

Automated architecture review of aec31c38b8ba — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.