feat: implement package management system with recipe support, regist…

[Grantlinkz]

…ry, and build synchronization.

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#111 "feat: implement package management system with recipe support, registry…"

head: 77f1d9f author: Grantlinkz ci: none reported · mergeable: CONFLICTING

Verdict: The shape is right — a remote index client in eBuild with HTTPS-only fetching,
strict name sanitisation, a size cap that survives a lying Content-Length, an atomic cache
replace, and an offline mode. Three things block it. A cached remote recipe silently
overwrites a project's own pinned url and checksum (reproduced). Nothing authenticates the
index, so checksum pins bytes without proving provenance — §10.1 asks for signatures. And
the default index URL points at a repository that does not exist, so the feature has never run
against a real index. The branch is 7 commits behind master, conflicts, and re-implements two
things already merged there.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	ebuild/packages/repository.py:118 vs :126-129	A cached remote recipe overrides the project's own pinned recipe, including its checksum. load_index is careful — if name not in self._index at :126, with the comment "Local recipe directory overrides remote index if already loaded". add_recipe_directory is not: :118 does an unconditional self._index[recipe.name] = info. load_all_sources calls it in the order project → shipped → cached remote (:157-160), so step 3 overwrites steps 1 and 2. Reproduced: a project shipping recipes/cjson.yaml (url: …/cjson-PROJECT.tar.gz, checksum: sha256:111…) alongside a cached ~/.ebuild/index/recipes/cjson.yaml (url: …/cjson-REMOTE.tar.gz, checksum: sha256:222…) resolves to version 9.9.9, the REMOTE url and the REMOTE checksum. So ebuild update-index can replace what a project has pinned in its own tree, and because the checksum is replaced too, fetcher.py's verification passes against the substituted archive. This is the §9.2 "reproducible lockfiles/manifests for production builds" guarantee, inverted.	Make add_recipe_directory respect precedence the way load_index does — either if recipe.name not in self._index there too, or give load_all_sources an explicit priority so the first source to define a name wins. Then add the test that would have caught it: three sources defining one package, asserting the project-local one resolves. That test does not exist today, which is why this is invisible to a green suite.
2	High	ebuild/packages/index_sync.py:127-232	The index is fetched and trusted with no authenticity or integrity check. sync() verifies the URL scheme is HTTPS, bounds the size, and parses JSON — then writes packages.json and derives recipe YAML from it. There is no signature over the index and no digest it is checked against. The per-package checksum is copied straight out of that same document (:207), so it pins the bytes of whatever url the same document supplied: it proves the download was not corrupted in transit, and nothing about where the package came from. --url (commands.py:1705) accepts any HTTPS origin with no allowlist or pinning. Master design §10.1 lists Integrity — "Hashes, signatures and provenance" as a component-contract field; §11 is the Registry and §15.1 requires "signed metadata and package provenance". Combined with finding 1, a single unauthenticated document can redirect and re-pin a project's dependencies.	Not all of it belongs in this PR, but the boundary has to be drawn here rather than left implicit. Minimum: detach the pin from the index — refuse to overwrite a checksum that a project-local recipe already states (finding 1 covers the mechanism), and record the index's own digest in packages.json so a changed index is visible. Then state in the docs that the index is unauthenticated, so nobody builds a release path on it before signing exists. The full answer is a detached signature over index.json verified against a key shipped with eBuild, which is §14.1's "integrate key management across eBoot, eSec, eOTA and release signing" extended to the registry, and is worth its own design discussion.
3	High	ebuild/packages/index_sync.py:32-34	DEFAULT_INDEX_URL points at a repository that does not exist. https://raw.githubusercontent.com/embeddedos-org/recipes/main/index.json — gh api repos/embeddedos-org/recipes returns 404, and so does the index.json path. It also names branch main, while .github/STANDARDS.md states every repo has exactly master and release; this branch's own history contains fix/setup-clones-master-not-main (#99) for the same mistake. Simulated the fresh-machine path: no cache plus a 404 gives IndexSyncError: Failed to fetch remote package index and no cache is available: HTTP Error 404: Not Found, and no cache is written, so the next run fails identically. Every test mocks urllib.request.urlopen, so the feature has never been exercised against a real index.	Either create embeddedos-org/recipes with an index.json on master and point the constant at master, or make the default empty and require --url until the registry exists — with ebuild search saying so rather than "Try running 'ebuild update-index'" (commands.py:1692-1696), which today is advice to run a command that cannot succeed. §28's claims policy applies: this is Planned, not Implemented, until the index it reads exists.
4	Medium	branch state	mergeStateStatus: DIRTY, mergeable: CONFLICTING. The branch sits on 562d28d (merge of #99); master is at e5d8052, 7 commits ahead. git apply of the PR diff onto master fails on ebuild/cli/commands.py and ebuild/packages/registry.py. Two of the conflicts are re-implementations of work already merged: master already has subprocess.run(argv, cwd=str(cwd), capture_output=True, text=True) and _NO_TESTS_MARKERS in test() (commands.py:2451, 2649), and master already has a working _board_config() with the docstring "A project that states its part's real capacity should not be measured against the reference part for its family." This PR's _board_config is an independent reimplementation that drops that docstring — merged as-is it would replace documented code with undocumented code.	Rebase on master and drop the test() and _board_config() changes entirely; they are already there and better documented. That should also shrink the diff and remove both conflicts.
5	Medium	pr body	The body is the unfilled template. No summary, - placeholders under Changes, every Testing and Pre-Submission box unchecked, no command output, no related issue — for 927 added lines that include a network-fetching subsystem and five new recipes. The brief treats an unsupported "verified" as a finding; a change of this size with no claim at all is harder to review, not easier. (The garbled type labels — eat, ix, efactor, est, uild — are not the author's doing; see Architecture conformance.)	Fill in Summary, Changes, and Testing with what was actually run. If the answer is "the 11 new tests", say that and say what is not covered — several of the findings above are honest gaps rather than mistakes, and naming them is faster than having a reviewer find them.
6	Medium	ebuild/packages/index_sync.py:186-190 vs repository.py:113-115	The cache is written before per-entry validation, and the reader does not sanitise. packages.json is atomically replaced at :186-190 with the raw downloaded array; only afterwards does the loop apply sanitize_package_name. So an entry the guard rejects still lands in the cached index, and load_index at repository.py:113 takes name = str(entry["name"]) with no sanitisation before building a PackageInfo. Reproduced: an index containing good, urlless and bad name! logs "Skipping unsafe package entry" and writes only good.yaml, while packages.json holds all three — and bad name! is then searchable. The path-traversal guard protects the recipe filenames and nothing else.	Filter the array before writing it: build a validated list in the loop and dump that, or apply sanitize_package_name in load_index too. A name that was refused once should not be reachable by a second path.
7	Medium	ebuild/packages/index_sync.py:130, 138; commands.py:1707	--force does nothing. force appears only in sync()'s signature and its docstring ("If True, re-download even if recently synced"); no line reads it, and there is no staleness check anywhere — sync() always re-downloads. The CLI advertises "Force refresh even if cache is up-to-date."	Either implement the staleness check the docstring describes (an mtime or a recorded fetch timestamp in packages.json) or remove both the parameter and the flag. A flag that is documented and inert is worse than an absent one.
8	Medium	commands.py:1712-1717; index_sync.py:229-232	ebuild update-index reports success and exits 0 when the sync failed. On any network error with a cache present, sync() returns a normal (count, message) tuple, and the CLI calls log.success(msg) and returns. The message text does say "Network sync failed (…); fell back to cached index", but it is rendered as a success and the exit status is 0, so a CI step that runs ebuild update-index before a build cannot tell a fresh index from a stale one. .ai/tooling.md: "Exit non-zero on failure, always."	Return the fallback distinguishably — a third element, or a dedicated exception the CLI catches to log.warn and exit non-zero (or 0 only under --offline, where using the cache is the request rather than a fallback).
9	Low	ebuild/packages/index_sync.py:203-232	Two smaller edges in the same block. (a) synced_count counts entries that produced nothing: the recipe is only written if recipe_dict["url"] (:215), but synced_count += 1 runs regardless. Reproduced — "Successfully synchronized 2 packages" with one recipe file on disk. (b) The except (URLError, HTTPError, OSError, TimeoutError) at :229 spans the cache writes too, so a disk-full while writing packages.json is reported as "Network sync failed (…)" and falls back to the stale cache, blaming the network for a local fault. §9.2 asks for actionable diagnostics.	(a) Move the increment inside the if, or count written and skipped separately and report both. (b) Narrow the try to the urlopen/read, and let cache-write failures surface as themselves.
10	Low	ebuild/packages/index_sync.py:216-221	recipe = _parse_recipe(recipe_dict) is assigned and never read — it is used only for its exception — and the unvalidated recipe_dict is what gets written to disk (:218). Whatever _parse_recipe normalises or defaults is validated and then discarded, so the cached YAML is the raw remote shape rather than the canonical one. _parse_recipe is also a module-private name imported across a module boundary (:26).	yaml.safe_dump(asdict(recipe), …) — write the thing that passed validation. If PackageRecipe needs a public constructor for this, add one rather than importing the underscore.
11	Low	commands.py:1, 871, 1621, 1641, 1687 etc.	An undisclosed encoding change rides along: # -*- coding: utf-8 -*- is added at :1 and six em-dashes in user-facing strings become ASCII hyphens ("ebuild — A unified embedded OS build system." → "ebuild - …", log.header("ebuild — Package Registry") → "ebuild - …", f" — {recipe.description}" → f" - …"). Counted: 39 em-dashes on master, 33 here — so 33 remain and the CLI's own output becomes inconsistent within one file. The pattern (a coding cookie plus selective em-dash loss) is the signature of a cp1252 editor round-trip rather than an intended change.	Revert all of it. Python 3 source is UTF-8 by default, so the cookie is noise, and the visual identity of the CLI output is not this PR's subject.
12	Low	pytest.ini:27	-p no:faker is added to addopts with no comment, in a file that comments every other section. Checked: faker is not imported anywhere under tests/ or ebuild/, and it is not installed here — so it changes nothing today and looks like a leftover from the author's environment. Recording it because a global test-runner flag arriving inside a feature PR is the shape worth catching even when this instance is harmless.	Drop it, or keep it with a one-line comment naming the conflict it avoids.
13	Low	ebuild/packages/repository.py:151-155 (search)	The license parameter shadows the builtin. Harmless in this scope, but the module already uses lic_filter for the same thing at the CLI boundary (commands.py:1670).	license_filter, matching the CLI.

Test coverage gaps (not scored separately; they are why findings 1, 6 and 7 are invisible):
the 11 new tests cover sanitisation, offline mode, insecure-URL rejection, corrupted JSON and
network fallback-with-cache — good choices — but there is nothing for source precedence, the
MAX_INDEX_SIZE_BYTES cap, a lying Content-Length, duplicate index entries, url-less
entries, or --force. Every network test mocks urlopen, so nothing exercises a real fetch.

Architecture conformance

Master design §10 (component and manifest system), §10.1 (component contract — Identity,
Compatibility, Dependencies, Capabilities, Permissions, Resources, Integrity, Compliance),
§11 and §11.1 (Registry and artifact types), §9.1–9.2 (eBuild engine and SDK design rules),
§15.1 (signed metadata and package provenance), §14.1, §21 tiers and §21.1 split policy.
Tier placement conforms; the component contract is only partly satisfied.

Placement is right. A remote index client in ebuild is Tier 1 – Foundation reading a
Tier 4 – Developer Ecosystem service, which is the direction §5.1 permits: eBuild "understands
the complete graph but is not a runtime dependency", and §11 names ebuild search mqtt /
ebuild add embeddedos/mqtt as the CLI surface for exactly this. §9.1's engine diagram already
puts "Packages / Registry" inside eBuild. Nothing in the diff points up a tier, and §21.1 is
not triggered — no new repository is proposed, and embeddedos-org/recipes would be data, not
a subsystem.

Where §10.1 is not met. The contract has eight fields; the recipe schema this PR caches
covers Identity, Dependencies and part of Compliance (license), and reduces Integrity to a
transit checksum with no signature or provenance (finding 2). Compatibility — "EmbeddedOS
API/ABI, architecture, SoC and target constraints" — and Resources — "Flash/RAM/storage" — are
absent from recipe_dict (index_sync.py:203-214) and from PackageInfo
(repository.py:26-40) entirely. For an embedded package manager those are the fields that
decide whether a package can go in an image at all; §10's own worked example carries
resources: flash_max / ram_max. That is a gap to name now, while the schema is new and
cheap to extend, rather than after recipes exist in the wild. Not scored as a finding because
this PR does not claim to implement the full contract — but the docs it adds should say which
fields are and are not carried.

Adjacent, not this PR's doing: .github/PULL_REQUEST_TEMPLATE.md on origin/master is
corrupted, which is why finding 5's type labels read eat/ix/efactor/est/uild. cat -A
shows - [ ] ^Leat — a literal formfeed where \feat was written, and the same for \fix
(FF), \refactor (CR), \test (TAB), \build (BS). Present in eBoot, ebuild, eos and
EoSim (two control characters each); the org-level template in embeddedos-org/.github is
correct. .github/STANDARDS.md says repos that ship no override inherit the org file, so
the fix is to delete the four local copies or repair them. Worth an issue against the org: the
template that tells contributors the Conventional Commit type names currently shows five
mangled ones, in the four most active repos.

Verified by running:

git apply of the PR diff onto origin/master
  -> error: ebuild/cli/commands.py: patch does not apply
  -> error: ebuild/packages/registry.py: patch does not apply       (finding 4)
git rev-list --count pr111..master -> 7                              (finding 4)
master already has: commands.py:2451 capture_output=True, text=True
                    commands.py:2649 _NO_TESTS_MARKERS
                    commands.py     _board_config() with its docstring and body

load_all_sources precedence, project + shipped + cached-remote all defining cjson:
  resolved version 9.9.9 · url …/cjson-REMOTE.tar.gz · checksum sha256:222…
  -> the cached remote recipe won                                    (finding 1)

sync() over an index of {good, urlless, "bad name!"}:
  "Skipping unsafe package entry: Invalid package name 'bad name!'"
  reported count: 2 · recipe files written: ['good.yaml']
  entries in packages.json: 3   ("bad name!" cached and searchable)  (findings 6, 9a)

gh api repos/embeddedos-org/recipes            -> 404 Not Found
gh api …/recipes/contents/index.json           -> 404 Not Found
fresh machine, no cache, default URL:
  IndexSyncError: Failed to fetch remote package index and no cache is available:
                  HTTP Error 404: Not Found      · cache file exists: False   (finding 3)

grep force  -> index_sync.py:130 (signature), :138 (docstring) only    (finding 7)
grep faker  -> no hits under tests/ or ebuild/; module not installed    (finding 12)
em-dashes in commands.py: master 39 · pr111 33                          (finding 11)
docs/architecture.md: build/orchestrator.py -> build/dispatch.py — correct,
  ebuild/build/ holds dispatch.py and no orchestrator.py

Worth crediting, because they are the parts that are easy to get wrong: HTTPS-only enforcement
(:151-155); response.read(MAX + 1) after the Content-Length check, so a lying header does
not defeat the cap; temp_json.replace() for an atomic cache swap; sanitize_package_name
with a strict allowlist rather than a blocklist; and fetcher.py:53-56 already refusing a
recipe with no checksum, so an empty checksum field cannot silently skip verification. The
docs/architecture.md correction is a real fix to a stale diagram, not churn.

Proposed changes

In order, because the first four gate the rest:

1. Rebase on master; drop the test() and _board_config() changes as already merged
   (finding 4). This removes both conflicts.
2. Make add_recipe_directory respect source precedence, and add the three-source test
   (finding 1).
3. Point DEFAULT_INDEX_URL at something that exists, or make it empty and say so in
   ebuild search's empty-state text (finding 3).
4. Fill in the PR body (finding 5).
5. Filter packages.json before writing it and sanitise in load_index (finding 6).
6. Implement or remove --force (finding 7); make the fallback exit non-zero (finding 8).
7. Findings 9–13 are small and can travel together.
8. Separately: state in docs/dependency-management.md that the index is unauthenticated and
   which §10.1 fields the recipe schema does not carry (finding 2, and the §10.1 note above).

No fix PR opened. Every finding is on this PR's branch, which the brief puts out of bounds, and
the branch needs a rebase before anything else is worth doing to it.

Not checked

• No CI has run on this head at all. actions/runs?head_sha=77f1d9f2… returns
  total_count: 0 — not even a queued-and-unapproved run, unlike #109 and #110 which have
  action_required runs. commits/77f1d9f2…/status is {"state":"pending","count":0}. So
  nothing in this PR has been verified by the project's pipeline, and the PR body claims
  nothing either.
• pytest is not installed in this environment, so none of the 11 new tests was run. Every
  result above comes from importing the modules directly and driving them, with
  urllib.request.urlopen mocked where the network would be reached. Whether the suite passes
  is unknown.
• No real network fetch. Findings 2 and 3 rest on the 404 from gh api plus a simulated
  HTTPError; I did not attempt to fetch the URL itself.
• The five new recipes' checksums were not verified. recipes/{cjson,lvgl,nanopb,tinyusb,unity}.yaml
  each carry a sha256: for an upstream tarball. Confirming those would mean downloading five
  archives from the network, which this run did not do. They are pins on third-party code and
  someone should check them before merge — a wrong one fails closed, but a copied-from-elsewhere
  one would not.
• No package was actually fetched, built, or installed. fetcher.py and builder.py were read
  where finding 2 depends on them, not exercised.
• ebuild search and ebuild update-index were driven through their library layer, not through
  the click CLI, so argument parsing, --json output shape and exit codes were reasoned from
  the source rather than observed.
• The local ebuild clone was left alone — the sync step reported it dirty (4 files, on
  branch v90), and the PR head was not present locally. I cloned it to /tmp with
  git clone --shared --no-checkout and fetched pull/111/head there, so the user's working
  tree, index and refs were never written to.

---

Automated architecture review of 77f1d9f22daf — 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.