fix(packages): add PackageRecipe serialization

[OshanYelena]

Summary

Fix package index synchronization failing with AttributeError when caching validated package recipes.

IndexSyncManager.sync() calls PackageRecipe.to_dict(), but PackageRecipe did not implement that method. This PR adds canonical recipe serialization using the existing YAML schema and adds regression coverage.

Type of Change

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

Changes

• Added PackageRecipe.to_dict() to serialize package recipes using the canonical YAML schema.
• Added a regression test verifying that serialization uses external YAML keys such as package and build rather than internal model fields such as name and build_system.

Testing

• Unit tests pass
• Integration tests pass
• Manual testing performed
• New tests added for new functionality

Tested with:

python -m pytest tests/unit/test_recipe.py tests/unit/test_index_sync.py -v
python -m pytest

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

[srpatcha]

Review — ebuild#124 "fix(packages): add PackageRecipe serialization"

head: 598c338 author: OshanYelena ci: none reported (mergeStateStatus: BLOCKED)

Verdict: The right fix for a real AttributeError, and of the three open PRs adding this method it is the only one whose output round-trips losslessly — I verified that parse_recipe(yaml.safe_load(yaml.safe_dump(r.to_dict()))) reconstructs an equal PackageRecipe, install_args included. One blocking defect: the new test file has no trailing newline, which fails the Lint (ruff) step that gates every later CI step.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	tests/unit/test_recipe.py:21	W292 No newline at end of file. ci.yml:58 runs ruff check . as an ungated step before yamllint, mypy and Run test suite, so this single character reddens the whole matrix and skips every test — the exact failure mode #122 exists to clear. Verified: ruff 0.16.5 check . on this head reports 5 errors, the 4 already on origin/master plus this one.	Add a trailing newline. ruff check --fix tests/unit/test_recipe.py does it.
2	Medium	ebuild/packages/recipe.py:92-107	Three open PRs add PackageRecipe.to_dict() and they do not agree. #119 returns dataclasses.asdict(self), emitting internal field names (name, build_system) into the cached recipe YAML. #132 emits the canonical schema but omits install_args entirely, so that field is silently dropped on every write through index_sync.py:354. This PR's version is the correct one — canonical external keys, all twelve fields, exact round-trip. Whichever lands first makes the other two conflicting or redundant.	Land this one and close #119; ask #132 to drop its recipe.py hunk rather than keep its lossy variant. Worth saying so on all three PRs so the decision is made once.
3	Low	ebuild/packages/recipe.py:92	Every key is emitted unconditionally, so a recipe with no description, license, checksum or patches writes description: '', license: '', checksum: '', patches: [] into the cached YAML under self.recipes_dir. Harmless — parse_recipe treats absent and empty identically — but it makes the cached files noisier than the hand-written recipes in the repo, and an empty checksum round-trips as a recipe that PackageFetcher.fetch() refuses to download, which reads as a pin that was deliberately cleared rather than one that was never set.	Optional. If you want the cached form to match the hand-written form, drop falsy optional keys — but keep install_args, url, version, package and build unconditional. Do not adopt #132's version to get this; it loses data.
4	Low	PR body, Pre-Submission Checklist	Two claims are not supportable as written. "Code compiles without warnings (-Wall -Wextra -Werror for C)" is ticked on a Python-only diff — there is nothing for those flags to apply to. And "New tests added for new functionality" is ticked in the Testing section and unticked in the Pre-Submission checklist, for the same one test. Per .ai/reviewer.md an unsupported claim is itself the finding, even a low-stakes one.	Untick the C-compiler box; tick the tests box once, consistently. Paste the actual pytest output — the body names the commands but shows no result.

Architecture conformance

Conforms. §21 places ebuild in Tier 1 — Foundation, and ebuild/packages/recipe.py is the recipe format for external dependencies, which belongs there. §5.1 is not engaged: to_dict adds no import beyond what the module already has (Dict, Any are imported at :15) and introduces no dependency in any direction.

§10.1 and §23.2 are the relevant clauses. §23.2 names "Package format — versioned .epkg/.eapp metadata schema" as a compatibility contract, and this method is the first place that schema gets written rather than only read. The keys chosen (package, version, url, checksum, build, dependencies, patches, configure_args, build_args, install_args, description, license) match what parse_recipe at :126-147 accepts as the preferred spelling, with the legacy name/build_system/depends aliases correctly not emitted. That is the right call: the aliases stay readable, the canonical form is what gets written.

On API compatibility (brief item 8): the change is purely additive — a new method on an existing dataclass, no signature or field altered — so there is no migration path to state and the PR's silence on it is correct.

Proposed changes

1. ruff check --fix tests/unit/test_recipe.py — one trailing newline, and the gate is clear.
2. Extend the new test to assert the round trip, not just the key names. The current assertions would still pass against an implementation that dropped install_args (which is precisely #132's bug), so they do not cover the property that matters:

   def test_package_recipe_to_dict_round_trips():
       recipe = PackageRecipe(
           name="demo", version="1.0.0",
           url="https://example.com/demo.tar.gz",
           build_system="cmake", dependencies=["dep"],
           install_args=["--prefix=/opt"], patches=["fix.patch"],
       )
       assert parse_recipe(yaml.safe_load(yaml.safe_dump(recipe.to_dict()))) == recipe

3. Resolve the three-way overlap per finding 2 before any of them merges.

Not checked

• pytest — NOT RUN. pytest is not importable on this host and the local ebuild clone has a dirty tree, left untouched per the rules of engagement. The body's claims that tests/unit/test_recipe.py, tests/unit/test_index_sync.py and the full suite pass are unverified. I confirmed only that the AttributeError cause is real — index_sync.py:354 calls recipe.to_dict() and origin/master's PackageRecipe has no such method.
• mypy — NOT RUN. Not installed. Dict[str, Any] is consistent with the module's existing annotations but the type check itself did not run.
• yamllint — NOT RUN. Not installed; no YAML in the diff.
• CI — no checks reported on fix/package-recipe-serialization. GitHub returns no check runs for this branch, so finding 1 has no CI evidence behind it; it comes from running ruff locally. mergeStateStatus is BLOCKED, which I read as required reviews outstanding rather than a red check, but I did not confirm which branch-protection rule is blocking.
• I did not verify that the 9 tests/unit/test_index_sync.py failures reported on master are all attributable to this missing method; I read the call site, I did not run the tests.

Verified locally: ruff 0.16.5 check . on a git archive export of head 598c3384 — 5 errors, the new one being W292 tests/unit/test_recipe.py:21. Round trip verified by executing parse_recipe/to_dict against that same export with PyYAML: twelve keys emitted, reconstructed recipe compares equal, install_args preserved.

---

Automated architecture review of 598c3384a386 — 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.

[srpatcha]

Thanks — of the three to_dict() proposals open right now (#119, #124, #132) yours is the most complete: canonical package:/build: keys, install_args included, lists copied, and it round-trips through parse_recipe(). The nine test_index_sync failures go away on your branch.

Two things before I can approve:

• tests/unit/test_recipe.py:21 — the file has no trailing newline (ruff check . reports W292). Please add the newline, and the # SPDX-License-Identifier: MIT header CONTRIBUTING.md asks for on new files.
• • Consider putting the test in the existing tests/ebuild/test_package_recipe.py and asserting parse_recipe(recipe.to_dict()) == recipe, which is the property index_sync.py:354 depends on.
    Heads-up for coordination: #132 restores #111's to_dict() as part of a wider "make master green" fix and I've approved it to land first. Once it merges this PR will conflict; the useful delta would then be adding install_args (and the round-trip test) on top of #132's version — that would be a quick approve.

[OshanYelena]

Thanks for the detailed review.

I’ve addressed the requested changes:

Added the SPDX-License-Identifier: MIT header.

Added the missing trailing newline.

Added a round-trip regression test using parse_recipe(recipe.to_dict()) == recipe, including install_args and patches to verify serialization is lossless.

I also re-ran the affected package/index-sync tests locally after the changes.

[srpatcha]

Review — ebuild#124 "fix(packages): add PackageRecipe serialization"

head: 8bbca73 author: OshanYelena ci: fail (policy / Policy / Linked Issue)

Verdict: Follow-up review. The one blocking defect from the previous report is fixed and verified; the new commits add a genuine round-trip regression test and are clean on their own merits. Two things still hold this up: the required policy / Policy / Linked Issue check is red because the body names no closing issue, and the three-way to_dict() overlap with #119 and #132 is still unresolved — both are still open.

Status of the previous findings

Prev #	Severity	Status	Evidence
1	High	Resolved in 8bbca739	ruff 0.16.5 check . on a git archive export of this head: 4 errors, all of them the pre-existing ones in tests/unit/test_ci_gate.py on origin/master. W292 tests/unit/test_recipe.py is gone; the file now ends ...restored == recipe\n.
2	Medium	Open	#119 (dkonlycomputer) and #132 (Kartikey1306) are both still open as of this run. Nothing in the new commits changes that, and nothing can — it is a decision, not a code change.
3	Low	Untouched	recipe.py:92-106 still emits all twelve keys unconditionally. This was flagged optional; not a blocker.
4	Low	Untouched	The PR body is unchanged (updatedAt moved but the checklist did not). See finding 2 below.

The two new commits were reviewed on their own merits and introduce no defect: the SPDX header matches the rest of tests/unit/, and test_package_recipe_to_dict_round_trips asserts exactly the property that distinguishes this implementation from #132's lossy one.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	PR body	Required check policy / Policy / Linked Issue fails. Job log: policy error: Pull request embeddedos-org/ebuild#124 must close at least one same-repository issue; no closing issues were recognized. Use Fixes #123, Closes #123, or Resolves #123 in the pull request body. This is the only check reported on the head and it is red, so the PR cannot merge regardless of the code being correct.	Open (or find) the issue tracking the IndexSyncManager.sync() AttributeError and add Fixes #<n> to the body. No code change needed.
2	Medium	ebuild/packages/recipe.py:92, #119, #132	Unchanged from the previous review, repeated only because it is still live: three open PRs add PackageRecipe.to_dict() and disagree. #119 returns dataclasses.asdict(self) (internal field names leak into cached YAML); #132 emits canonical keys but drops install_args, silently losing that field on every write through index_sync.py:354. This PR is the only one whose output round-trips. Whichever merges first makes the other two conflicting or redundant.	Land this one; close #119; ask #132 to drop its recipe.py hunk. The decision needs making once, on all three PRs.
3	Low	PR body, Pre-Submission Checklist	Still unsupported as written, per .ai/reviewer.md: "Code compiles without warnings (-Wall -Wextra -Werror for C)" is ticked on a Python-only diff, and "New tests added for new functionality" is [x] in Testing and [ ] in Pre-Submission for the same single test file. The comment says the tests were re-run locally but no output is shown.	Untick the C box, tick the tests box consistently, and paste the pytest output. For what it is worth I ran it for you — see below — so the claim is now true, it is just not evidenced in the body.

Architecture conformance

Conforms; unchanged from the previous review and re-checked against the mirror rather than recalled.

• §21 places ebuild in Tier 1 — Foundation. ebuild/packages/recipe.py is the external-dependency recipe format and belongs there.
• §5.1 is not engaged. to_dict adds no import (Dict, Any already imported at recipe.py:15) and creates no dependency in any direction, let alone an upward one.
• §23.2 ("Package format — versioned .epkg/.eapp metadata schema") is the live contract. Verified on this head that the emitted keys — package, version, description, license, url, checksum, build, dependencies, patches, configure_args, build_args, install_args — are exactly the preferred spellings parse_recipe accepts at :126-147, with the legacy name/build_system/depends aliases correctly not emitted. Reading stays permissive, writing stays canonical. That is the right shape for a format contract.
• Brief item 8 (API/wire compatibility): purely additive — a new method, no signature or field altered. There is no migration path to state, so the body's silence is correct here.

Proposed changes

1. Add Fixes #<issue> to the body. That is the whole of the remaining blocker on this PR.
2. Resolve the #119/#124/#132 overlap before any of the three merges (finding 2).
3. Optional, and only if you want cached recipes to read like the hand-written ones: drop falsy optional keys in to_dict, keeping package, version, url, build and install_args unconditional. Do not adopt #132's version to get this — it loses data.

Verification I ran

Against a git archive export of head 8bbca739 (the local ebuild clone has a dirty tree and was left untouched; the PR head was fetched read-only):

Check	Result
ruff 0.16.5 check .	4 errors — all pre-existing on origin/master in tests/unit/test_ci_gate.py (E402 ×4). Previous finding 1's W292 is gone.
pytest tests/unit/test_recipe.py tests/unit/test_index_sync.py -q	24 passed in 0.19s (in a clean venv built from pyproject.toml[dev]).
Round trip, executed	parse_recipe(yaml.safe_load(yaml.safe_dump(r.to_dict()))) == r → True, with install_args and patches set. Twelve keys emitted.

Not checked

• Full pytest suite — NOT RUN. I ran only the two modules the PR body names. The body's python -m pytest claim over the whole suite is unverified.
• mypy — NOT RUN. Not installed on this host. Dict[str, Any] is consistent with the module's existing annotations, but the type check itself did not execute.
• yamllint — NOT RUN. Not installed; there is no YAML in the diff.
• The ci.yml matrix — NOT RUN as CI. GitHub reports only the policy check on this head; Lint (ruff), mypy, yamllint and Run test suite have no check runs. My ruff and pytest results are local, on an export, not CI evidence.
• I did not re-verify that all 9 previously reported test_index_sync.py failures on master trace to the missing method; the 22 tests in that module pass on this head, which is consistent with it but is not the same claim.
• mergeStateStatus is BLOCKED and reviewDecision is CHANGES_REQUESTED. I did not determine whether the outstanding change request is the one this PR's new commits answer.

---

Automated architecture review of 8bbca7392d90 — 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.

[srpatcha]

Review — ebuild#124 "fix(packages): add PackageRecipe serialization"

head: 8bbca73 author: OshanYelena ci: fail (policy / Policy / Linked Issue)

Verdict: Follow-up review. The one blocking defect from the previous report is fixed and verified; the new commits add a genuine round-trip regression test and are clean on their own merits. Two things still hold this up: the required policy / Policy / Linked Issue check is red because the body names no closing issue, and the three-way to_dict() overlap with #119 and #132 is still unresolved — both are still open.

Status of the previous findings

Prev #	Severity	Status	Evidence
1	High	Resolved in 8bbca739	ruff 0.16.5 check . on a git archive export of this head: 4 errors, all of them the pre-existing ones in tests/unit/test_ci_gate.py on origin/master. W292 tests/unit/test_recipe.py is gone; the file now ends ...restored == recipe\n.
2	Medium	Open	#119 (dkonlycomputer) and #132 (Kartikey1306) are both still open as of this run. Nothing in the new commits changes that, and nothing can — it is a decision, not a code change.
3	Low	Untouched	recipe.py:92-106 still emits all twelve keys unconditionally. This was flagged optional; not a blocker.
4	Low	Untouched	The PR body is unchanged (updatedAt moved but the checklist did not). See finding 2 below.

The two new commits were reviewed on their own merits and introduce no defect: the SPDX header matches the rest of tests/unit/, and test_package_recipe_to_dict_round_trips asserts exactly the property that distinguishes this implementation from #132's lossy one.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	PR body	Required check policy / Policy / Linked Issue fails. Job log: policy error: Pull request embeddedos-org/ebuild#124 must close at least one same-repository issue; no closing issues were recognized. Use Fixes #123, Closes #123, or Resolves #123 in the pull request body. This is the only check reported on the head and it is red, so the PR cannot merge regardless of the code being correct.	Open (or find) the issue tracking the IndexSyncManager.sync() AttributeError and add Fixes #<n> to the body. No code change needed.
2	Medium	ebuild/packages/recipe.py:92, #119, #132	Unchanged from the previous review, repeated only because it is still live: three open PRs add PackageRecipe.to_dict() and disagree. #119 returns dataclasses.asdict(self) (internal field names leak into cached YAML); #132 emits canonical keys but drops install_args, silently losing that field on every write through index_sync.py:354. This PR is the only one whose output round-trips. Whichever merges first makes the other two conflicting or redundant.	Land this one; close #119; ask #132 to drop its recipe.py hunk. The decision needs making once, on all three PRs.
3	Low	PR body, Pre-Submission Checklist	Still unsupported as written, per .ai/reviewer.md: "Code compiles without warnings (-Wall -Wextra -Werror for C)" is ticked on a Python-only diff, and "New tests added for new functionality" is [x] in Testing and [ ] in Pre-Submission for the same single test file. The comment says the tests were re-run locally but no output is shown.	Untick the C box, tick the tests box consistently, and paste the pytest output. For what it is worth I ran it for you — see below — so the claim is now true, it is just not evidenced in the body.

Architecture conformance

Conforms; unchanged from the previous review and re-checked against the mirror rather than recalled.

• §21 places ebuild in Tier 1 — Foundation. ebuild/packages/recipe.py is the external-dependency recipe format and belongs there.
• §5.1 is not engaged. to_dict adds no import (Dict, Any already imported at recipe.py:15) and creates no dependency in any direction, let alone an upward one.
• §23.2 ("Package format — versioned .epkg/.eapp metadata schema") is the live contract. Verified on this head that the emitted keys — package, version, description, license, url, checksum, build, dependencies, patches, configure_args, build_args, install_args — are exactly the preferred spellings parse_recipe accepts at :126-147, with the legacy name/build_system/depends aliases correctly not emitted. Reading stays permissive, writing stays canonical. That is the right shape for a format contract.
• Brief item 8 (API/wire compatibility): purely additive — a new method, no signature or field altered. There is no migration path to state, so the body's silence is correct here.

Proposed changes

1. Add Fixes #<issue> to the body. That is the whole of the remaining blocker on this PR.
2. Resolve the #119/#124/#132 overlap before any of the three merges (finding 2).
3. Optional, and only if you want cached recipes to read like the hand-written ones: drop falsy optional keys in to_dict, keeping package, version, url, build and install_args unconditional. Do not adopt #132's version to get this — it loses data.

Verification I ran

Against a git archive export of head 8bbca739 (the local ebuild clone has a dirty tree and was left untouched; the PR head was fetched read-only):

Check	Result
ruff 0.16.5 check .	4 errors — all pre-existing on origin/master in tests/unit/test_ci_gate.py (E402 ×4). Previous finding 1's W292 is gone.
pytest tests/unit/test_recipe.py tests/unit/test_index_sync.py -q	24 passed in 0.19s (in a clean venv built from pyproject.toml[dev]).
Round trip, executed	parse_recipe(yaml.safe_load(yaml.safe_dump(r.to_dict()))) == r → True, with install_args and patches set. Twelve keys emitted.

Not checked

• Full pytest suite — NOT RUN. I ran only the two modules the PR body names. The body's python -m pytest claim over the whole suite is unverified.
• mypy — NOT RUN. Not installed on this host. Dict[str, Any] is consistent with the module's existing annotations, but the type check itself did not execute.
• yamllint — NOT RUN. Not installed; there is no YAML in the diff.
• The ci.yml matrix — NOT RUN as CI. GitHub reports only the policy check on this head; Lint (ruff), mypy, yamllint and Run test suite have no check runs. My ruff and pytest results are local, on an export, not CI evidence.
• I did not re-verify that all 9 previously reported test_index_sync.py failures on master trace to the missing method; the 22 tests in that module pass on this head, which is consistent with it but is not the same claim.
• mergeStateStatus is BLOCKED and reviewDecision is CHANGES_REQUESTED. I did not determine whether the outstanding change request is the one this PR's new commits answer.

---

Automated architecture review of 8bbca7392d90 — 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.