fix(packages): read ebuild.lock back, so it pins what it records

[Kartikey1306]

Stacked on #132 (master is red without it). Review 4054494..ba4e248 for this change alone: three commits (restacked once, onto #132's rebase 4054494 after master moved to c37e995; f4788f2..c26d9f9 was the same content on the previous base -- 51d4642/3a941c6/ba4e248 are 90010fd/bf2a776/c26d9f9 replayed, no conflicts, none of #122's files are touched here). 90010fd is the fix (6 files); bf2a776 answers the first review's three findings; c26d9f9 answers the second review's findings 4 and 5 (ebuild/packages/lockfile.py, tests/unit/test_resolver.py). bf2a776 covered (ebuild/packages/lockfile.py, ebuild/packages/resolver.py, ebuild/cli/commands.py, docs/dependency-management.md, the two test files, CHANGELOG.md).

Problem (ebuild #145)

ebuild.lock was written after every package resolution and never read. Nothing called Lockfile.load(), and _install_packages constructed the Lockfile only after PackageResolver.resolve() had already chosen versions — the code, in order:

resolved = resolver.resolve(requested)
…
lockfile = Lockfile(lock_path)
…
lockfile.lock(resolved); lockfile.save()

So an unpinned package resolved to the newest recipe on every machine and every run, while an ebuild.lock sat in the project claiming to pin it. docs/architecture.md ("records exact resolved versions for reproducibility") and the CHANGELOG's "guaranteeing reproducible builds (§9.2)" described a file that had no effect on resolution.

Change

PackageResolver.resolve(requested, lockfile=None); the CLI loads the lock before resolving and passes it in, and rewrites it from the result as before.

Precedence, in order:

1. an explicit request — the statement of intent; it outranks the lock, and the lock is rewritten afterwards;
2. the lock — pins every package the request leaves open;
3. the newest version in the registry.

Two refusals, because silently resolving to something else is exactly what the lock exists to prevent:

• a locked version no recipe provides → ResolveError: ebuild.lock pins 'zlib' at v9.9.9, and no recipe provides that version. Restore the recipe, or request the version you want explicitly, or delete ebuild.lock to resolve afresh.
• a locked entry whose recorded URL or checksum no longer matches the recipe of that version → refused. The version reproduces the name; the lock is there to reproduce the bytes.

Lockfile.load() keeps only well-formed entries, so a hand-edited file cannot become a KeyError inside the resolver.

Tests (Verified; five of the six resolver tests fail against the previous resolver, and the CLI test fails against the previous _install_packages)

• test_lockfile_pins_a_package_the_request_leaves_open — zlib at 1.2.13 and 1.3.0; without a lock → 1.3.0, with the lock → 1.2.13
• test_an_explicit_request_outranks_the_lock
• test_a_locked_version_no_recipe_provides_is_an_error_not_a_fallback
• test_the_lock_notices_a_recipe_whose_bytes_changed — and the control with the real checksum resolves
• test_a_lock_entry_the_request_does_not_reach_is_ignored
• test_load_drops_malformed_entries
• tests/unit/test_lockfile_cli.py — _install_packages hands the resolver the lock that was on disk and rewrites it afterwards

Full suite on this head: ruff, yamllint, mypy (mypy . --ignore-missing-imports --no-strict-optional --exclude '^(layers|core|promo)/') clean; pytest 696 passed / 1 skipped.

Review follow-up (bf2a776)

Finding	What changed
1 (P2) — a corrupt lock crashed with a yaml.scanner.ScannerError traceback, and this PR made that reachable	Lockfile.load() raises LockfileError for a lock that is not YAML (ebuild.lock is not valid YAML: ... Fix it, or delete it to resolve afresh.) and for one it cannot open (OSError: a directory, a permission). _install_packages converts it to ResolveError at the one load site, so all three callers print [error] ... as they already do for the two refusals. Tests: through Lockfile.load() (unterminated quote; a directory in place of the file), through _install_packages(), and end to end through ebuild build with the real project fixture shape -- exit 1, the message, no Traceback.
2 (P3) — build was recorded and never compared	Lockfile.CHECKED_FIELDS = (("url","url"), ("checksum","checksum"), ("build","build_system")) is the one list both lock()'s readers use; _check_locked_bytes() iterates it (field name → attribute, since the attribute is build_system). test_the_lock_notices_a_recipe_whose_build_system_changed (with the control), and test_every_field_the_lock_records_is_one_the_resolver_compares, which asserts set(lock entry) - {"version"} == set(CHECKED_FIELDS) so a fifth recorded field cannot go uncompared again.
3 (P3) — no user-facing documentation	docs/dependency-management.md gains an ebuild.lock section: what it records, request → lock → newest precedence, that drift in a locked field is refused, commit it, how to upgrade (name the version, or delete the file), and how a corrupt vs. wrong-shape lock is treated.

Negative control for the follow-up: with 90010fd's lockfile.py, resolver.py and commands.py restored, all six new tests fail (the CLI one prints the ScannerError traceback).

Second review follow-up (c26d9f9)

Finding	What changed
4 (P3) -- a lock that is not UTF-8 escaped as a UnicodeDecodeError traceback	except (yaml.YAMLError, UnicodeDecodeError) with the same message and remedy. test_load_refuses_a_lock_that_is_not_utf8: a 0xff byte and a UTF-16 BOM both raise LockfileError; against bf2a776 the first raises the UnicodeDecodeError.
5 (P3) -- a registry binding assigned only for the file its call writes	Bare call with a comment saying what it is for.

pytest 697 passed / 1 skipped on c26d9f9; ruff, yamllint, mypy clean. On ba4e248 (the restack): ruff, yamllint, mypy (CI's invocation) clean; pytest 694 passed / 1 skipped, with 3 not run here -- test_end_to_end_build_from_outside_produces_the_binary, test_editing_a_header_triggers_a_rebuild, test_measures_a_real_binary link a real C binary and this host's linker is currently broken (ld: tapi error: malformed file … MacOSX27.0.sdk … unknown architecture, identical at the base 4054494); CI covers them.

Closing issue

Fixes #145

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.92308% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ebuild/cli/commands.py	90.00%	0 Missing and 1 partial ⚠️
ebuild/plugins/__init__.py	50.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[srpatcha]

Review — ebuild#146 "fix(packages): read ebuild.lock back, so it pins what it records"

head: 90010fda3d4fd990a6d2fd9a6d150a6d1188354a author: Kartikey1306 ci: pass (30 green, 0 red; only Create GitHub Release skipped)

Verdict: First look at f4788f2..90010fd (one commit, 6 files) as the body asks; the other 12
files belong to #132. The fix is right and I verified all four behaviours through the real CLI, not
through the tests: without a lock zlib resolves to 1.3.0, with a lock to 1.2.13, a lock naming a
version no recipe provides is refused, and a lock whose recorded URL no longer matches the recipe
is refused — both with clean exit 1 and an error that says what to do. Three findings, one of
which is the mirror image of the PR's best quality: the two failure paths the author designed
have excellent diagnostics, and the one they did not think about prints a fifty-line PyYAML
traceback.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	ebuild/packages/lockfile.py:37-56; reachable from ebuild/cli/commands.py:89	A corrupt ebuild.lock now crashes the build with a traceback, and this PR is what makes that reachable. load() calls yaml.safe_load(f) with no handler. Until this commit nothing called load(), so a malformed lock was inert; now every configure, build and package runs it. Verified end to end with the real CLI on a project whose lock had one unterminated quote: exit 1 with a 50-line yaml.scanner.ScannerError traceback ending in click and PyYAML frames, after the log had already printed Resolving packages.... The call sites catch FileNotFoundError, ConfigError, RecipeError, ResolveError, FetchError and BuildError (commands.py:1364-1371); yaml.YAMLError is in none of them. IsADirectoryError and PermissionError escape the same way — I reproduced both out of load() directly, and neither is a FileNotFoundError, so neither is caught. What makes this worth a finding rather than a nit is the comment at :50-51: "a hand-edited or truncated file must not turn into a KeyError deep inside the resolver." A truncated file is precisely the case that still raises, one frame earlier than the one the comment guards. The sanitising dict comprehension below it handles a well-formed YAML file of the wrong shape — I confirmed packages: [zlib], zlib: 'a-string', zlib: [1,2,3] and an empty file all yield {} cleanly — which is the easier half. This repo already has the pattern one file over: commands.py:407-411 does except yaml.YAMLError as e: log.error(f"… is not valid YAML: {e}"); raise SystemExit(1).	Wrap the read and raise the project's own error type so the existing handlers catch it: try: raw = yaml.safe_load(f) except yaml.YAMLError as e: raise ResolveError(f"{self.lock_path} is not valid YAML: {e}. Fix it, or delete it to resolve afresh.") — and catch OSError around the open() for the directory and permission cases. ResolveError is already handled at all three call sites and already produces [error] Error: …, so this lands the corrupt-lock case in the same shape as the two refusals this PR wrote by hand. One test per case; the truncated fixture is two lines.
2	Low (P3)	ebuild/packages/lockfile.py:76; ebuild/packages/resolver.py:186-200	build is recorded in the lock and never read — the exact shape of the defect this PR exists to fix. Lockfile.lock() writes four fields per package: version, url, checksum, build. _check_locked_bytes() compares ("url", "checksum"). So a recipe that keeps its name, version, URL and checksum and changes build: cmake to build: make passes the lock unchallenged and produces a different artifact from the same source. The PR's own argument — "the version reproduces the name; the lock is there to reproduce the bytes" — applies: the build system is part of what turns the source bytes into the installed bytes. Small and contained; I am rating it Low because a recipe edit of that kind is rarer than a URL change, not because the reasoning is weaker.	Either add "build" to the tuple at :196 (comparing against recipe.build_system, which is the attribute name — getattr(recipe, "build") would AttributeError, so this needs a field→attribute map, not a fourth string), or stop recording it in lock(). Recording a field nobody reads is what #145 was about.
3	Low (P3)	docs/architecture.md:183; docs/ generally	ebuild.lock has no user-facing documentation, and this PR is what makes it authoritative. grep -rn "ebuild.lock" docs/*.md README.md returns zero matches; the only description anywhere is architecture.md:183, "Lockfile (lockfile.py) — records exact resolved versions for reproducibility", inside an internal component list. That sentence was false before this PR and is true after, so nothing is made wrong (brief item 11 is not tripped) — but the change introduces a workflow a user will need and cannot look up: an existing lock now silently outranks "give me the newest", and the way to move forward is to delete the file or name the version. Today that appears only inside two error strings and the CHANGELOG. §9.2 asks for "reproducible lockfiles/manifests for production builds" and §25.2 for "excellent errors with suggested fixes"; the errors are excellent, and there is nowhere to read about the feature before hitting one.	A short docs/ section, or a paragraph in the packages guide: what ebuild.lock is, the request → lock → newest precedence, that it is rewritten after every resolution, that it should be committed for reproducible builds, and how to upgrade (delete it, or pin explicitly). Ten lines.

Recorded as a limit, not a finding: _check_locked_bytes() skips any field the lock recorded as
empty, and lock() records recipe.checksum verbatim — so for a recipe with no checksum, the lock
pins a URL, which is mutable content, not bytes. That is the correct behaviour given what the
recipe provides and the docstring says so ("Only fields the lock recorded are compared"); it does
mean the body's "the lock is there to reproduce the bytes" holds only for recipes that carry a
checksum. Worth a sentence wherever finding 3's documentation lands.

What this PR gets right

The precedence order is the right one and is argued rather than asserted: the request is a
statement of intent, the lock is a record, and the record is rewritten from the result — so
"explicit request outranks the lock" is not an arbitrary tie-break. Both refusals are errors
rather than fallbacks, which is the whole point; a lockfile that silently resolved to something
else would be worse than no lockfile. And the error strings are genuinely good — each one names
the file, the package, both values, and three ways out. _locked_entries() filtering out
explicitly-pinned names before resolution starts, rather than checking precedence at each node, is
also the cleaner of the two available shapes.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/ebuild-146 on refs/pull/146/head, using the
run's shared venv. The ebuild checkout has a dirty working tree — TASKS.md,
ebuild/cli/integration.py, tests/ebuild/test_integration_initramfs_security.py and an untracked
smart-sensor/ — and I left it exactly as found, re-checked after all work. Nothing was
committed or pushed. The CLI probes ran against throwaway projects under /tmp.

Check	Result
pytest tests/ -q	PASS — 690 passed, 1 skipped, 6.21s — matches the body exactly
pytest tests/unit/test_resolver.py tests/unit/test_lockfile_cli.py -q	PASS — 18 passed
ruff 0.16.5 check .	PASS — All checks passed!
End to end, real CLI, two recipes (zlib 1.2.13 and 1.3.0), no lock	Packages to install: zlib v1.3.0 — newest, as before
Same project, lock pinning 1.2.13	Packages to install: zlib v1.2.13 — the headline claim, confirmed against the binary, not the unit tests
Lock pinning v9.9.9, which no recipe provides	exit 1, [error] Error: ebuild.lock pins 'zlib' at v9.9.9, and no recipe provides that version. Restore the recipe, or request the version you want explicitly, or delete ebuild.lock to resolve afresh.
Lock pinning v1.2.13 with a stale URL	exit 1, [error] Error: ebuild.lock pins 'zlib' v1.2.13 with url '…/OLD.tar.gz', but the recipe for that version now has '…/zlib-1.2.13.tar.gz'. …
Corrupt lock (unterminated quote), real CLI ebuild configure	exit 1, 50-line yaml.scanner.ScannerError traceback — finding 1
Lockfile.load() shape probes	packages: [zlib], zlib: 'string', zlib: [1,2,3], empty file → all {} clean ✅ · truncated, tab-indented, unclosed flow mapping → ScannerError/ParserError ❌ · lock path a directory → IsADirectoryError ❌ · mode 000 → PermissionError ❌
Negative control — locked = {} forced in resolve(), rebuilt	3 of 18 FAIL: test_lockfile_pins_a_package_the_request_leaves_open, test_a_locked_version_no_recipe_provides_is_an_error_not_a_fallback, test_the_lock_notices_a_recipe_whose_bytes_changed. The tests genuinely pin the new behaviour. (The other two lock tests assert the absence of an effect and pass either way — correct for controls.) Restored; git status clean.
Was anything weakened?	No. No test disabled, no assertion removed, no lint rule loosened, no || true, no permission widened. resolve() gains an optional parameter, so existing callers still work.
Public API change (brief item 8)	PackageResolver.resolve(requested, lockfile=None) — additive and backward compatible. The user-visible behaviour change (an existing lock now outranks "newest") is stated in the body and the CHANGELOG, with the migration path (delete the lock, or request a version). Item 8 satisfied; finding 3 is that it is stated nowhere a user would look.

Architecture conformance

Conforms. §21 places ebuild in Tier 1 — Foundation; every file is inside the owning repo and
§21.1 is not engaged. §5.1 dependency direction holds and the new import is downward within the
package: packages/resolver.py → packages/lockfile.py → packages/recipe.py, with no cycle
(resolver already reached recipe via registry). "eBuild understands the complete graph but is
not a runtime dependency" is untouched — none of this ships to a device. The clause this PR serves
is §9.2, "Reproducible lockfiles/manifests for production builds", and it is the difference
between claiming that and having it: before this commit ebuild.lock was written and never read,
so §9.2 was satisfied by a file, not by a behaviour, and §28's evidence policy would have rated it
Planned described in the present tense. It is now Implemented with functional tests, which
is what §28 requires for that word. §23.2's "eBuild project format — backward-compatible
project/manifest migration policy" is respected: the lock format is unchanged, lockfile_version: 1
still, and old locks are read by the new code.

Proposed changes

Before merge, small:
  lockfile.py:42-43   wrap yaml.safe_load in except yaml.YAMLError -> ResolveError,
                      and the open() in except OSError; the existing call-site
                      handlers then catch it                              (finding 1)
  tests/              a truncated-lock fixture and an unreadable-lock case

After, not blocking:
  lockfile.py:76      check "build" too, or stop recording it             (finding 2)
  docs/               ten lines on what ebuild.lock is and how to upgrade (finding 3)

No fix PR opened. Finding 1 is Medium, and the brief's autofix rule is High-only. It is also
in lockfile.py's load() as this branch rewrites it — a branch cut from origin/master, which
is what fix-start.sh produces, would be patching a function with a different body. It belongs to
the author, in this PR.

Not checked

• mypy — NOT RUN. Not installed in the scratch env. The body's "mypy clean" is unverified
  locally; the green Test (Python 3.x, os) legs imply the Type check (mypy) step passed, since
  ci.yml runs it before the tests with no continue-on-error. Inferred. Note that finding
  2's suggested fix would need a field→attribute map rather than a fourth string in the tuple, and
  mypy would not catch the naive version because getattr() is untyped.
• yamllint — NOT RUN. Not installed.
• No package was actually fetched or built. My end-to-end runs used https://example.invalid
  URLs and stopped at resolution, which is the code this PR changes. I did not verify that a
  locked resolution then fetches, checksums and builds the pinned archive — that path is unchanged
  by this diff but is where a lockfile's value is finally realised.
• Concurrency — NOT EXAMINED. _install_packages builds packages across threads
  (commands.py:104-118), and lockfile.save() runs after. I did not check what two ebuild
  processes in one project directory do to ebuild.lock; save() is a plain truncating write with
  no locking, and that was true before this PR as well.
• pytest on Python 3.10 and 3.11 — NOT RUN. My venv is 3.12. CI covers them.
• The #132 half of the 18-file diff was not reviewed here — it is reviewed separately in this
  same run at head f4788f2c. If #132 changes, the 690-passed figure above no longer describes
  what will merge.
• mergeStateStatus is BLOCKED — REVIEW_REQUIRED plus the #132 stack.

---

Automated architecture review of 90010fda3d4f — 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.

[Kartikey1306]

All three taken, in bf2a776 (range is now f4788f2..bf2a776; the body has a "Review follow-up" table).

1 — a corrupt lock crashed with a traceback, and this PR made that reachable. Agreed, including the point about my own comment: a truncated file did still raise, one frame earlier than the one the comment guarded. Lockfile.load() now raises LockfileError for a lock that is not YAML — ebuild.lock is not valid YAML: <parser message>. Fix it, or delete it to resolve afresh. — and for one it cannot open (OSError: the directory and permission cases). I did not raise ResolveError from lockfile.py directly, because resolver.py imports Lockfile and that would be a cycle; the one load site in _install_packages converts LockfileError to ResolveError, so all three callers print [error] ... exactly as they do for the two refusals. Tests at each layer: Lockfile.load() with an unterminated quote and with a directory in place of the file, _install_packages() with the same lock, and end to end through ebuild build with the project fixture shape test_build_failure_output.py uses — exit 1, the message, and "Traceback" not in output. Against 90010fd the end-to-end one prints the ScannerError traceback you saw.

2 — build recorded and never compared. Taken your way, with the field→attribute map since the attribute is build_system: Lockfile.CHECKED_FIELDS = (("url", "url"), ("checksum", "checksum"), ("build", "build_system")), next to lock(), is the one list _check_locked_bytes() iterates. test_the_lock_notices_a_recipe_whose_build_system_changed covers build: cmake → make with the same URL and checksum (and the control with the real build system resolves), and test_every_field_the_lock_records_is_one_the_resolver_compares asserts that everything lock() writes except the version key is in CHECKED_FIELDS, so a fifth recorded field cannot go uncompared again.

3 — no user-facing documentation. docs/dependency-management.md has an ebuild.lock section now: what it records, request → lock → newest precedence, that drift in a locked field is refused and why, that it should be committed, how to upgrade (name the version in build.yaml, or delete the file), and how a corrupt lock (error) differs from a wrong-shape one (sanitised entry by entry).

Gates on bf2a776: ruff, yamllint, mypy with CI's exact invocation clean; pytest 696 passed / 1 skipped (690 + 6). Line endings held: ebuild/cli/commands.py 2935 → 2940 CR bytes, one per added line; everything else LF.

[srpatcha]

Review — ebuild#146 "fix(packages): read ebuild.lock back, so it pins what it records"

head: bf2a776 author: Kartikey1306 ci: pass — 30 green, 0 red; only Create GitHub Release skipped

Verdict: Follow-up on one new commit, bf2a776, which answers all three findings from
ebuild-146-90010fda.md. All three are resolved, and I confirmed each by execution rather
than by reading: the build-field comparison refuses a drifted recipe through the real CLI, the
corrupt-lock path reports [error] … is not valid YAML with no traceback, and the six new tests
all fail against 90010fd's implementation — the author's own negative control, reproduced.
One residual case of finding 1 survives the fix and I reproduced it end to end, plus one
cosmetic nit in a new test. Both are Low.

Follow-up on the previous review

Prior #	Sev	Status	Evidence at this head
1	Medium (P2)	Resolved for the cases it named; one residual — see new finding 4	lockfile.py:61-73 wraps the open()/yaml.safe_load() in except yaml.YAMLError and except OSError, raising a new LockfileError; commands.py:89-95 converts it to ResolveError at the single load() call site, which the three handlers at commands.py:1078, :1271 and :1374 already catch. Declining to raise ResolveError from lockfile.py to avoid a resolver↔lockfile cycle is the right call. Reproduced through the real CLI: unterminated quote → exit 1, [error] Package error: … ebuild.lock is not valid YAML: …, no traceback. A directory in place of the file → LockfileError: … could not be read.
2	Low (P3)	Resolved, and generalised further than asked	Lockfile.CHECKED_FIELDS = (("url","url"), ("checksum","checksum"), ("build","build_system")) at lockfile.py:39-46 is the single list both sides use, with the field→attribute map the previous review said would be needed. resolver.py:193 iterates it. Verified through the real CLI, not only the tests: a recipe with build: make against a lock recording build: cmake, same version/URL/checksum → exit 1, [error] Package error: ebuild.lock pins 'zlib' v1.3.0 with build 'cmake', but the recipe for that version now has 'make'. …; with the lock corrected to make the same project resolves ([info] Packages to install: zlib v1.3.0) and proceeds to fetch. test_every_field_the_lock_records_is_one_the_resolver_compares asserts set(lock entry) - {"version"} == set(CHECKED_FIELDS), which is the part that stops this recurring.
3	Low (P3)	Resolved	docs/dependency-management.md:363-387 — a 25-line ebuild.lock section covering what it records, request → lock → newest precedence, that drift in a locked field is refused and why, that it should be committed, how to upgrade (name the version, or delete the file), and that a corrupt lock errors while a wrong-shape one is sanitised entry by entry. It is the only file under docs/ that mentions ebuild.lock and it is reachable from README.md:91. docs/architecture.md:183 ("records exact resolved versions for reproducibility") is now true rather than aspirational.

The previous review's "recorded as a limit, not a finding" — that a recipe with no checksum
leaves the lock pinning a mutable URL — is not covered by the new documentation. Not a
finding; noted so it is not mistaken for covered.

Review of the new commit on its own merits

#	Severity	File:line	Finding	Recommended fix
4	Low (P3)	ebuild/packages/lockfile.py:61-73	The corrupt-lock fix misses the non-UTF-8 case, which reaches the terminal as the same traceback finding 1 was about. open(..., encoding="utf-8") is inside the try, but UnicodeDecodeError is a ValueError — neither yaml.YAMLError nor OSError — so it escapes LockfileError, escapes the ResolveError handlers, and propagates out of the command. Reproduced end to end with a real ebuild build on a project whose ebuild.lock contains a 0xff byte: exit 1 preceded by a 44-line UnicodeDecodeError traceback ending in yaml/reader.py … <frozen codecs>, after the log had printed Resolving packages... — byte for byte the shape finding 1 described. A UTF-16-saved lock (the BOM alone does it) behaves identically. Rated Low rather than Medium because the cases a hand-edit or truncation actually produces — bad YAML syntax, an unreadable path — are now handled, and save() only ever writes ASCII; this needs a foreign encoding, a bad binary merge, or a partial overwrite.	One word: except (yaml.YAMLError, UnicodeDecodeError) as e: at :63, keeping the same message (PyYAML's own reader raises yaml.reader.ReaderError for a decode failure it detects itself, so the two belong in one clause). A test alongside test_load_refuses_a_lock_that_is_not_yaml — path.write_bytes(b"packages:\n zlib: {version: '\xff'}\n") — is two lines.
5	Low (P3)	tests/unit/test_resolver.py:277-287	registry = make_registry_with_checksums(...) is assigned and then overwritten four lines later by registry = PackageRegistry(); the first call is there only for the zlib.yaml it writes, which the next statement appends build: make to. The name says the opposite of what the call is for, and a reader has to run it to find out. Ruff does not flag it because the name is later reused.	Drop the binding: make_registry_with_checksums(tmp_path, {...}) on its own line, or name it _. Cosmetic.

Nothing was weakened. No test disabled, no assertion removed, no lint rule loosened, no
|| true, no permission widened — checked across the whole follow-up diff. LockfileError is a
new public name in ebuild.packages.lockfile; it is purely additive, Lockfile.load()'s
signature is unchanged, and the behaviour change (a previously-uncaught traceback becomes a
clean error) is stated in the CHANGELOG entry bf2a776 rewrites. Brief item 8 satisfied.

Architecture conformance

Conforms, re-checked against the follow-up diff. §21 places ebuild in Tier 1 — Foundation;
every file is inside the owning repo, so §21.1 is not engaged. §5.1 dependency direction holds
and no new edge is introduced: resolver.py already imported Lockfile for Lockfile.FILENAME
and now also reads Lockfile.CHECKED_FIELDS from it; lockfile.py still imports only
packages/recipe.py, so there is no cycle — which is why the author converts LockfileError
to ResolveError in the CLI rather than raising ResolveError from lockfile.py. "eBuild
understands the complete graph but is not a runtime dependency" is untouched; none of this ships
to a device.

The clause this PR serves is §9.2, "Reproducible lockfiles/manifests for production builds",
and bf2a776 tightens it: the recorded build system is now part of what the lock reproduces,
which is the difference between reproducing a package's name and its installed artifact.
§25.2's "excellent errors with suggested fixes" now holds on the failure path that previously
printed a PyYAML traceback — for every input except finding 4's. §23.2's "eBuild project format
— backward-compatible project/manifest migration policy" is respected: lockfile_version: 1 is
unchanged and an old lock without a build field is still read, because _check_locked_bytes()
skips a field the lock recorded as empty.

No new proposal appended. Nothing here reveals a gap in the master design; §9.2 named the
requirement correctly and this is the implementation catching up to it.

Proposed changes

Small, before or after merge:
  lockfile.py:63   except (yaml.YAMLError, UnicodeDecodeError) as e:   (finding 4)
  tests/unit/test_resolver.py  a two-line non-UTF-8 fixture beside
                   test_load_refuses_a_lock_that_is_not_yaml
  tests/unit/test_resolver.py:277  drop the discarded `registry =`      (finding 5)

Blocking nothing. #132 still has to land first; this is stacked on it.

No fix PR opened. Both findings are Low and the brief's autofix rule is High-only. Both are
also in files as this branch rewrites them, so a branch cut from origin/master — which is what
fix-start.sh produces — would be patching different bodies. They belong to the author, in
this PR.

Verification performed for this review

The ebuild checkout has a dirty working tree (TASKS.md, ebuild/cli/integration.py,
tests/ebuild/test_integration_initramfs_security.py modified, smart-sensor/ untracked) and
was not touched — no worktree, checkout, stash or reset; re-checked after all work. The PR
head was fetched read-only into refs/autoreview/pr146; everything below ran against
git archive bf2a7762 | tar -x into a temporary directory outside the repository, with
click/pyyaml/ninja resolved from pyproject.toml into a throwaway CPython 3.12
environment. CLI probes ran against throwaway projects under /tmp. Nothing was committed or
pushed.

Check	Result
pytest 9.1.1 tests/ -q	PASS — 696 passed, 1 skipped in 5.28s. Exactly the figure the body claims (690 + 6)
ruff 0.16.5 check .	PASS — All checks passed!
mypy . --ignore-missing-imports --no-strict-optional --exclude '^(layers|core|promo)/' — CI's exact invocation from ci.yml:68	PASS — "Success: no issues found in 108 source files". The previous review recorded mypy as NOT RUN; it is run here
Negative control — lockfile.py, resolver.py, commands.py restored to 90010fd, the six new tests re-run	6 failed, 0 passed. test_the_lock_notices_a_recipe_whose_build_system_changed, test_every_field_the_lock_records_is_one_the_resolver_compares, test_load_refuses_a_lock_that_is_not_yaml, test_load_refuses_a_lock_it_cannot_open, test_install_packages_reports_a_corrupt_lock_as_a_resolve_error, test_build_command_prints_a_corrupt_lock_without_a_traceback. The author's claimed control, reproduced
Real CLI — lock records build: cmake, recipe says build: make, same version/URL/checksum	exit 1, [error] Package error: ebuild.lock pins 'zlib' v1.3.0 with build 'cmake', but the recipe for that version now has 'make'. The lock exists to notice this; … — finding 2 confirmed against the binary
Real CLI, control — lock corrected to build: make	resolves: [info] Packages to install: zlib v1.3.0, proceeds to fetch (then fails on example.invalid, as designed)
Real CLI — ebuild.lock with an unterminated quote	exit 1, [error] … ebuild.lock is not valid YAML: …, no traceback — finding 1's headline case confirmed fixed
Lockfile.load() — lock path is a directory	LockfileError: … could not be read: [Errno 21] Is a directory …
Lockfile.load() — lock with a NUL byte	LockfileError: … is not valid YAML: unacceptable character …
Lockfile.load() — lock with a 0xff byte, and a UTF-16-encoded lock	UNCAUGHT UnicodeDecodeError in both — finding 4
Real CLI — ebuild build on a project with a 0xff byte in ebuild.lock	exit 1 with a 44-line UnicodeDecodeError traceback printed to the terminal — finding 4 confirmed end to end
Lockfile.load() call sites	exactly one, commands.py:90, and it is the one wrapped
ResolveError handlers reached from it	commands.py:1078, :1271, :1374 — all three catch it
git diff --stat 90010fd bf2a776	7 files, +196/−10; CHANGELOG.md, docs/dependency-management.md, commands.py, lockfile.py, resolver.py, and the two test files
grep -rln "ebuild.lock" docs/ README.md	docs/dependency-management.md only; reachable from README.md:91
CI on this head (checks.txt)	30 pass, 0 fail, incl. policy / Policy / Linked Issue, CodeQL, Analyze (Python), 9 Test (3.10/3.11/3.12 × ubuntu/macos/windows) legs, 11 EoSim targets, CI Gate, Simulation Sanity Gate
PR metadata	mergeable: MERGEABLE, mergeStateStatus: BLOCKED, reviewDecision: REVIEW_REQUIRED

Not checked

• yamllint — NOT RUN. Not installed. The body's yamllint claim is unverified locally; this
  commit changes no YAML, so nothing in it is exposed by that gap.
• The line-ending claim was not verified. The author reports ebuild/cli/commands.py
  2935 → 2940 CR bytes, one per added line. I did not count CR bytes; git archive normalises
  under .gitattributes, so the export is the wrong artifact to measure it from.
• No package was actually fetched or built. Every end-to-end probe used
  https://example.invalid and stopped at resolution or at the download error. The path from a
  locked resolution through fetch → checksum → build is unchanged by this diff and remains
  unverified by me.
• pytest on Python 3.10 and 3.11 — NOT RUN. 3.12 only; CI covers the others and is green.
• Concurrency — still NOT EXAMINED, as in the previous review. lockfile.save() is a plain
  truncating write with no locking and no error handling of its own: a read-only project
  directory would traceback out of save() the way load() used to. That is pre-existing, not
  introduced here, and I did not probe it.
• Codecov reports 96.92% patch coverage, 2 lines missing (commands.py 1 partial,
  plugins/__init__.py 1 — the latter is #132's file). I did not chase which branch in
  commands.py is uncovered; on inspection the only new branch there is the except LockfileError arm, which two of the new tests exercise.
• The #132 half of the 19-file diff is not re-reviewed here — reviewed separately at head
  f4788f2c. If #132 changes, the 696-passed figure no longer describes what will merge.
• mergeStateStatus: BLOCKED — REVIEW_REQUIRED plus the #132 stack. Land #132 first, as
  the body says.

---

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

[Kartikey1306]

Both taken, in c26d9f9 (range now f4788f2..c26d9f9; the body has a second follow-up table).

Finding 4 — non-UTF-8 lock. except (yaml.YAMLError, UnicodeDecodeError), same message and remedy, with a comment on why the codec's error is not the parser's. test_load_refuses_a_lock_that_is_not_utf8 covers your 0xff byte and a UTF-16 save (the BOM alone), both LockfileError; against bf2a776 the first test raises the UnicodeDecodeError you reproduced end to end. Thanks for the yaml.reader.ReaderError note — it is a YAMLError subclass, so the one clause covers both ways a decode can fail.

Finding 5 — the overwritten registry binding. Bare call now, with a one-line comment saying it is there for the zlib.yaml it writes.

pytest 697 passed / 1 skipped; ruff, yamllint and mypy (CI's invocation) clean. Still on f4788f2.

[Kartikey1306]

Status at the current head — my last comment ended "Still on f4788f2", and that is no longer true.

It was written against c26d9f9. master then moved to c37e995, #132 was rebased onto it as 4054494, and this branch was replayed on top: 90010fd/bf2a776/c26d9f9 are now 51d4642/3a941c6/ba4e248, no conflicts, none of #122's files touched. Review 4054494..ba4e248 — three commits. The PR body already carries this mapping; the comment did not, which is why this one exists.

Re-verified at ba4e248 just now, with CI's own invocations:

• ruff check . — clean
• yamllint . — clean
• mypy . --ignore-missing-imports --no-strict-optional --exclude '^(layers|core|promo)/' — no issues in 108 source files
• pytest tests/ — 697 passed, 1 skipped

Same counts as the pre-restack run, which is the expected outcome for a replay. The single skip is test_windows_installer.py, Windows cmd.exe validation — it does not run on this host and did not run before.

One thing worth stating plainly, because it cost me a wrong reading here first: three tests that shell out to a compiler (test_end_to_end_build_from_outside_produces_the_binary, test_editing_a_header_triggers_a_rebuild, test_measures_a_real_binary) fail on this machine with ld: tapi error: malformed file … MacOSX27.0.sdk/usr/lib/libSystem.B.tbd. That is a broken SDK in the local Xcode 27 install, not this branch — pointing SDKROOT at MacOSX26.5.sdk makes all three pass, and the counts above are from that shell. CI is unaffected; it builds on ubuntu-22.04/macos-latest runners with their own toolchains, and all five legs are green on this head.

The findings from the last round are unchanged and still covered: test_load_refuses_a_lock_that_is_not_utf8 (the 0xff byte and the UTF-16 BOM, both LockfileError), and the bare registry call.