fix: run the signing suite in CI and keep the boot log head consistent on write failure

[dhruv-joshi15]

Summary

Three small, independent fixes to test and build hygiene:

1. A test suite that had never run in CI now runs. tests/unit/test_sign_image.py was skipping all 14 of its cases because its dependency was declared nowhere.
2. The boot log no longer corrupts its ring on a failed flash write. eos_boot_log_append() advanced the head past a slot it never wrote.
3. Two avoidable host build warnings are cleared, moving toward the zero-warning standard in CONTRIBUTING.md.

Nothing here changes boot behaviour on a working device. The value is that failures now surface instead of passing silently.

Each item is a separate commit and can be reviewed or dropped independently.

Context

eBoot is a two-stage bootloader: Stage-0 brings up hardware, Stage-1 scans the A/B slots, verifies an image (SHA-256 / Ed25519 / anti-rollback), and jumps to it. core/ holds the platform-agnostic logic that builds and tests natively on the host; boards/ and stage0/ hold architecture-specific code that does not.

That split makes the host test suite the main safety net for core/ — it is the only part CI can actually execute. Both substantive fixes here are about that net having a hole: one suite was not running at all, and one failure path had a fixture but no test.

What changed

Commit	Scope	Files
b1b6c05	fix(ci) — signing tests execute again	requirements.txt, .github/workflows/ci.yml, tests/unit/test_requirements.py
dc32112	fix(core) — boot log head consistency	core/boot_log.c, tests/unit/test_boot_log.c
7521325	fix(build) — host build warnings	core/boot_menu.c, tests/CMakeLists.txt

7 files, +222 / −4. No changes to secure_boot.c, image_verify.c, ed25519_verify.c, recovery.c, keystore.c, stage0/, boards/, jump_app.c, or the root CMakeLists.txt.

---

1. The signing test suite was never executing in CI

Problem

tests/unit/test_sign_image.py guards itself with:

pytest.importorskip("cryptography", reason="signing tools require 'cryptography'")

cryptography is imported by tools/sign_image.py and tools/eos_sign.py, but was declared neither in requirements.txt nor by the CI job that runs pytest — that job installed a hand-written list, pip3 install pytest pytest-cov. Because importorskip degrades to a skip rather than a failure, the suite sat quietly in the skipped column and the job still reported success.

Impact

The 14 cases that never ran are the ones pinning the v2 signed-header wire format: that the Ed25519 signature covers the whole header prefix, that entry_addr, load_addr, image_size, image_version, sig_type and hdr_version are each bound to it, that clearing EOS_IMG_FLAG_HASH_SHA256 is rejected, and that an unsigned image fails --verify.

All 14 pass today — nothing is broken. The exposure is that a regression in imgpack.py or sign_image.py would have merged green.

Fix

• Declare the dependency: cryptography>=41.0 in requirements.txt, following the file's existing name>=major.minor convention.
• Install from the requirements file instead of a parallel list: pip3 install -r requirements.txt pytest-cov. pytest-cov stays separate because it is a CI-only coverage plugin, not a repository dependency.
• Add tests/unit/test_requirements.py so this cannot lapse again.

The importorskip guard stays — a contributor without the signing tools should still get a skip, not a hard error. The new test is what makes the skip impossible in CI.

Regression guard

Three checks, layered specific to general, written as static text parsing in the style of test_cmake_test_registration.py — no cmake, compiler, network, GitHub API or YAML dependency:

1. test_cryptography_is_declared_in_requirements — pins the specific dependency by name.
2. test_every_importorskip_dependency_is_declared — cross-references every importorskip(...) against requirements.txt, so a future suite with an undeclared guard is caught too.
3. test_jobs_that_run_pytest_install_the_repository_requirements — scans every workflow, finds each job running pytest over tests/, and requires -r requirements.txt. It names the offending job in the failure message.

Both failure modes were confirmed by breaking the fix and watching the tests fail:

# with cryptography removed from requirements.txt
AssertionError: test_sign_image.py skips itself when these modules are missing, but
requirements.txt does not declare them, so they will never be installed and the
suite will never run: ['cryptography']

# with the CI install line reverted
AssertionError: these jobs run pytest over tests/ but never install from
requirements.txt, so a declared dependency is absent at run time and the suites
that need it skip while the job still passes: ['ci.yml:test']

---

2. eos_boot_log_append() advanced the head past a slot it never wrote

Problem

eos_hal_flash_write(addr, &entry, sizeof(entry));   /* return value discarded */

log_head = (log_head + 1) % EOS_BOOT_LOG_MAX;       /* advances regardless */

When the write fails the head advances anyway. The slot is left as erased flash but skipped permanently.

Impact

log_head is persisted in the boot control block and reloaded by eos_boot_log_init() on the next boot, so the gap survives the reset. eos_boot_log_read() then returns all-0xFF erased flash that a reader cannot distinguish from a real entry, and the slot is never reused. The consumers affected are eos_fw_read_boot_log() and the recovery log-retrieval command, both of which walk the ring from eos_boot_log_get_head().

This is a diagnostics and post-mortem reliability issue — nothing in the verification chain reads the boot log; attestation is separate, in secure_boot.c.

Reproduced before the fix

head after failed append = 2 (expected 1: the slot was never written)
  [FAIL] eos_boot_log_get_head() == 1
slot 1 event after next successful append = 0xFFFFFFFF (expected EOS_LOG_CONFIRM=0x00000009)
  [FAIL] e.event == (uint32_t)EOS_LOG_CONFIRM

Fix

Advance the head only after the write succeeds:

int rc = eos_hal_flash_write(addr, &entry, sizeof(entry));
if (rc != EOS_OK)
    return;

log_head = (log_head + 1) % EOS_BOOT_LOG_MAX;

eos_boot_log_append() returns void, so a failure still cannot be reported to the caller — but it can decline to advance. This mirrors what eos_boot_log_clear() in the same file already does for a failed erase, and the reasoning documented there applies equally: reporting success would let the next boot walk over entries that are still present.

No signature change, no header change, no new API, no new error code, no change to bootctl.

Regression test

test_append_does_not_advance_head_when_write_fails uses the write_result injection hook that already existed in the fixture but that no test had exercised. It asserts the head does not move, the target slot is still erased, the next successful append lands in that same slot, the head then advances normally, and the entry written before the failure is untouched.

Confirmed load-bearing by reverting the fix and rebuilding:

[FAIL] tests/unit/test_boot_log.c:257: eos_boot_log_get_head() == 1
... all 9 pre-existing tests PASS

The pre-existing suite genuinely did not cover this path.

---

3. Pre-existing host build warnings

Fixed in a separate commit, because CONTRIBUTING.md asks for a warning-clean -Wall -Wextra build.

core/boot_menu.c — two -Wunused-parameter warnings on port. The cause is worth recording: eos_hal_uart_write(port, data, len) is a compatibility macro in eos_hal.h that expands to eos_hal_uart_send(data, len) and discards the port, because the HAL exposes only a single-UART API. Honouring the port properly would mean adding per-port HAL calls across every board port, well outside this PR. Added (void)port; — the idiom already used throughout core/ — with a comment recording why it is ignored, so the warning goes without hiding the API gap.

tests/CMakeLists.txt — ld: warning: ignoring duplicate libraries: '../libeboot_core.a'. eboot_test_recovery named eboot_core eboot_stage1, but eboot_stage1 already links eboot_core PUBLIC, and the explicit copy sat ahead of the target needing it — the wrong order for a left-to-right archive scan — so CMake appended it again and the linker reported the duplicate. Verified with nm before removing it that there is no circular dependency: eboot_core requires zero symbols from eboot_stage1, while eboot_stage1 requires many from eboot_core. The transitive link covers it on GNU ld as well as ld64.

---

Testing

Before

CI-equivalent environment (pip install pytest pytest-cov, matching what ci.yml did), on unmodified master:

$ python -m pytest tests/ -q -rs
SKIPPED [1] tests/unit/test_sign_image.py:26: signing tools require 'cryptography'
19 passed, 1 skipped in 0.05s
exit 0          # green, with the signing suite never executed

After

Environment built the way CI now builds it (pip install -r requirements.txt pytest-cov):

$ python -m pytest tests/ -q
36 passed in 2.72s

36 = 19 pre-existing + 14 signing + 3 new guard. Zero skips.

Full results

Check	Result
Release configure + build	PASS, exit 0
Release ctest --no-tests=error	20/20 (100%)
Debug configure + build	PASS, exit 0
Debug ctest --no-tests=error	20/20 (100%)
pytest tests/	36 passed, 0 skipped
pytest tests/unit/test_sign_image.py	14 passed
pytest tests/unit/test_requirements.py	3 passed
ctest -R test_boot_log	11/11 (was 10/10)
New compiler warnings	none — output diffed against a build of unmodified master
Warning count	4 before → 1 after (the remaining one is deliberate; see below)

Contributors without cryptography installed still get a clean run with a graceful skip: 22 passed, 1 skipped.

cmake -B build -DCMAKE_BUILD_TYPE=Release -DEBLDR_BUILD_TESTS=ON -DEBLDR_BOARD=none
cmake --build build --parallel
ctest --test-dir build --output-on-failure --no-tests=error
pip install -r requirements.txt
python -m pytest tests/ -q

---

Everything found, and what was done about it

This came out of a targeted review of the host-testable parts of the tree. It was not an exhaustive audit, and it deliberately did not examine the verification, recovery or key-handling paths.

Fixed in this PR

#	Finding	Location	Why fixed here
1	Signing suite skipped in CI; dependency declared nowhere	requirements.txt, ci.yml	Reproducible, tiny diff, restores 14 tests, zero behaviour change
2	Ring head advances past a slot a failed write never reached	core/boot_log.c	Reproduced, two-line fix, precedent already set by eos_boot_log_clear()
3	Two -Wunused-parameter warnings	core/boot_menu.c	Violates the project's stated zero-warning standard; local, no redesign
4	Duplicate library on the link line	tests/CMakeLists.txt	Same standard; verified safe via symbol analysis

Found and deliberately left alone

#	Finding	Location	Why not touched
5	core/fdt_loader.c is in no CMake source list, so it is never compiled; it has no callers and no tests	CMakeLists.txt	#72 identified this and explicitly left it as "a separate module and a separate question". Enabling a module that has never been built or exercised deserves a maintainer decision and its own test coverage, not a one-line addition bundled into an unrelated PR.
6	max_attempts == 0 is interpreted two ways: eos_slot_needs_rollback() treats it as "rollback disabled" and test_slot_manager.c pins that, while eos_boot_policy_select() has no such guard so zero forces a rollback every boot — and docs/PRODUCTION_TESTING_REPORT.md records the latter as intended	core/boot_policy.c, core/slot_manager.c, docs	Both readings have in-repo support, so which is correct is a design decision, not a bug fix. Choosing one unilaterally would change rollback behaviour on real hardware. Happy to open an issue.
7	Dockerfile test-runner stage installs only pytest, then runs pytest tests/ — the same gap as finding 1	Dockerfile:23	Real, but no workflow builds the Dockerfile, so it is dormant. Kept out to keep this PR's scope tight; a one-line follow-up if wanted.
8	core/sha512.c and core/rollback.c are each listed twice in add_library(eboot_core ...)	CMakeLists.txt	Verified harmless: CMake deduplicates, and only one object of each is produced. Maintenance noise, not a defect — not worth touching the root build file for.
9	The valgrind foreach covers 16 of the 20 suites; test_ecc, test_storage, test_rollback and test_secure_boot get no valgrind target	tests/CMakeLists.txt	Would widen an already-separate cleanup commit, and valgrind could not be run locally to confirm the four suites pass under it. Better as its own change.
10	CONTRIBUTING.md says "19 C unit test suites" and "all 7 unit tests"; there are 20 registered, and test_secure_boot is missing from the table	CONTRIBUTING.md	Docs-only drift, unrelated to any change here. Belongs in a docs PR.
11	.github/PULL_REQUEST_TEMPLATE.md contains literal control characters that have eaten the first letter of feat, fix, refactor, test and build	PR template	Same — real and easy to fix, but unrelated to this PR.
12	core/keystore.c emits a #warning on every build	core/keystore.c:29	Intentional and left in place. It fires when a build embeds the RFC 8032 test-vector key as the trust anchor without EBLDR_PRODUCTION_KEY, and the comment above it says the warning is deliberate. It is doing its job. This is why the build reports 1 warning rather than 0.
13	tests/CMakeLists.txt uses CRLF line endings throughout, so git diff --check flags any added line in it	tests/CMakeLists.txt	Added lines match the file's existing endings and contain no real trailing whitespace. Normalising would produce a 148-line whitespace-only diff that buries the actual change.

---

Relationship to previous work

The history here is a sequence of correct changes that each left one seam; nothing below was done wrongly.

• fix(secure-boot): sign the whole image header, not just the payload hash #40 (8d8ce74) introduced tests/unit/test_sign_image.py along with the v2 signed-header format. The suite is correct and all of it passes. What did not follow was declaring cryptography in the repository requirements or the CI environment — requirements.txt was last changed in Implement authenticated recovery log retrieval #31 (d3ae185), three days before that suite existed, and no commit has ever named cryptography in requirements.txt or a workflow. This PR closes that gap.

• test: pin the .efw image header wire format (38 checks) #67 (ab5a4bf) rewrote the boot-log suite so it exercises core/boot_log.c rather than its own stubs, and added a fixture that can inject both erase and write failures. The erase path is guarded in eos_boot_log_clear() and covered by a test. The write path had the fixture hook but no test, and eos_boot_log_append() advanced the head unconditionally. This PR adds the missing test and the matching guard, following the pattern test: pin the .efw image header wire format (38 checks) #67 established.

For completeness on the boot log's provenance: #41 (cd9793a) touched only ci.yml and CMakeLists.txt, and #49 (e3b6d41) was a pure stage1/ → core/ rename with no content change. eos_boot_log_append() is byte-identical to its form in the initial v0.1.0 commit — the unconditional head advance has been there from the start and had only ever been moved.

---

Limitations

• CI has not run on this branch yet. All results above are from local out-of-tree builds on macOS / arm64 with Apple clang 15.0.0, Python 3.13.4 and CMake 4.4.3. Linux/GCC-12 and MSVC behaviour is expected but unverified here; the workflow change will first execute when this PR opens.
• cryptography>=41.0 is a conservative maintained-release floor, not an API requirement — the APIs used (Ed25519PrivateKey, Ed25519PublicKey, serialization, InvalidSignature) have been stable since 2.6. Manylinux wheels exist for the runner's Python, so no build toolchain should be needed.
• The workflow-scanning test parses YAML as text, deliberately, to avoid a PyYAML dependency in the guard itself. It would not understand dependencies installed via a composite action or reusable workflow, and fails loudly if it finds no pytest job at all.
• The boot-log fix is verified against the simulated-flash fixture, not real flash hardware. A failed write still loses that entry — the fix corrects ring consistency, not log completeness; changing that would require an API break.
• Cross-compilation, hardware validation, valgrind and cppcheck/clang-tidy were not run locally (no toolchains; valgrind is unavailable on arm64 macOS).

---

Recommended follow-ups

In rough order of value, all deliberately kept out of this PR:

1. Decide the max_attempts == 0 contract and align boot_policy.c, slot_manager.c and the docs behind one answer.
2. Decide whether core/fdt_loader.c is a live module; if so, wire it in together with tests.
3. Apply the same -r requirements.txt install to the Dockerfile test stage.
4. Refresh the CONTRIBUTING.md test counts and repair the PR template's mangled type list.
5. Extend the valgrind target list to the four uncovered suites.

Happy to open issues or follow-up PRs for any of these.

---

Checklist

• Builds clean in Release and Debug with no new warnings
• All existing tests pass — 20/20 C suites, 36 Python tests
• New tests added, and each confirmed to fail with the fix reverted
• No API, header or ABI change
• No changes to verification, recovery, keystore, stage0 or board code
• Commits follow Conventional Commits and are independently reviewable

[srpatcha]

Review — eBoot#91 "fix: run the signing suite in CI and keep the boot log head consistent on write failure"

head: 7521325 author: dhruv-joshi15 ci: none reported

Verdict: Three well-scoped fixes, split one per commit, with an unusually honest body that separates what was fixed from what was found and left alone. Two of the three I could verify statically and both check out. The problems are that no CI ran at all, so none of the extensive local evidence is corroborated; that the boot-log fix is right for an atomic write failure and leaves the partial-write case worse-shaped than it looks; and that one of the "warning cleanups" silences a diagnostic that was reporting a real functional gap.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	— (checks.txt is empty)	No CI check of any kind reported on this head. checks.txt is 0 bytes, and createdAt / updatedAt are 2026-09-01T17:33:08Z / 17:33:09Z — opened and never touched again. Every other PR in this batch carries 22-26 checks. So the entire verification table in the body — ctest 20/20 in Release and Debug, 36 passed under pytest, 14 passed for the signing suite, 11/11 for test_boot_log, "no new compiler warnings, output diffed against a build of unmodified master" — is a local run by the author with nothing independent behind it. That matters more than usual here because the PR changes core/boot_log.c, which is inside eboot_core and therefore inside the TCB, and because the tests/CMakeLists.txt change removes a library from a link line, which is exactly the class of change only a real build validates. The likely cause is benign — a first-time contributor whose workflow runs need maintainer approval, which dhruv-joshi15 appearing as a new author in this repo would explain — but the effect is a core/ change with zero executed evidence.	A maintainer approves the workflow runs so the 26 checks execute against this head, before anything else in this review is acted on. If the runs come back green, findings 2-4 are the remaining substance; if they do not, the local table needs re-examining rather than trusting.
2	Medium	core/boot_menu.c:56-67	The two (void)port; lines silence a warning that is telling the truth. Verified: include/eos_hal.h:163 defines #define eos_hal_uart_write(port, data, len) eos_hal_uart_send(data, len) and include/eos_hal.h:129 declares int eos_hal_uart_send(const void *buf, size_t len) — no port parameter anywhere. So eos_boot_menu_config_t::uart_port is inert: a board that configures the recovery menu onto UART 2 gets it on whichever UART the HAL's single-instance API happens to serve, silently. -Wunused-parameter was the only thing in the tree saying so, and this change removes it while leaving the gap. The run brief is explicit that "a disabled test, loosened lint, removed assertion, or widened permission is a finding regardless of the reason given", and a (void)x over a genuinely-ignored configuration parameter is that shape. To be fair to the author, the comment is candid about exactly this — "it starts being honoured the moment the HAL grows a per-port call" — which is why it is Medium rather than higher; the alternative fixes are just all better.	Fail closed instead of suppressing. Smallest version: in eos_boot_menu_init() (or wherever the config is accepted), if (cfg->uart_port != 0) return EOS_ERR_NOT_SUPPORTED; with a comment pointing at eos_hal.h:163, and mark the field /* unimplemented: the HAL exposes a single UART */ in its header. A board asking for something the HAL cannot do then hears about it at init rather than losing its menu output. Better but larger: give eos_hal_uart_send() a port argument and drop the compatibility macro — that is the actual fix and probably its own PR.
3	Medium	core/boot_log.c:44-52	The fix is correct for a write that fails atomically and leaves the partial-write case in a worse shape than the new test suggests. eos_boot_log_entry_t is {timestamp, event, slot, detail}, four uint32_t with no validity marker or CRC, and readers distinguish "unused" by convention — the new test itself asserts entry.event == 0xFFFFFFFFu for an erased slot. Now consider a NOR write that programs timestamp and then fails before event: the head correctly stays put, the slot still reads as erased because event is untouched, and the next append rewrites the same address. NOR programming can only clear bits, so the second write's timestamp is ANDed with the residue of the first — a silently corrupted timestamp, while event, slot and detail land cleanly. A reader gets an entry that looks entirely valid with a wrong time. The new test cannot see this because its mock fails the write atomically (write_result = EOS_ERR_FLASH with no partial effect), so the retry lands on genuinely erased flash. §8.1 requires "Crash/health information available to update logic"; a plausible-looking wrong timestamp is worse for that than a missing entry.	Do not change the entry layout — test_entry_layout_is_stable exists precisely because host tooling and application firmware parse this struct, so adding a CRC word is a wire-format change. The smaller fix is to make the retry safe: before writing, read the target slot and, if it is not fully erased, advance the head past it rather than programming over it. Then extend the mock to fail after writing the first word so the partial case is actually covered — that is the test that distinguishes the two behaviours.
4	Low	core/boot_log.c:26 (signature)	With this fix a failed write is now silently discarded: eos_boot_log_append() returns void, so the caller cannot learn that a boot event — including EOS_LOG_BOOT_FAIL — never reached flash. Before the change the event was also lost but the head moved, which at least left a visible gap; now the loss is invisible. .ai/security.md is direct about "a security step whose result is discarded", and §8.1 makes boot health information an input to update logic. Pre-existing signature and the PR is a net improvement, but the author's own comment names the constraint ("append() cannot report a failure -- it returns void"), so it should not close as done.	Change the return type to int and propagate, then audit the call sites — most will legitimately ignore it, but the ones in the failure and rollback paths should not. Its own PR; eos_boot_log_clear() already returns a status, so the precedent is in the file.
5	Low	.github/workflows/ci.yml:39	Three PRs in this batch rewrite this one line — #88 appends cryptography, #90 appends pyyaml, and this one replaces the hand-written list with pip3 install -r requirements.txt pytest-cov. They will conflict pairwise. Worth saying because it is not a three-way tie: this PR's version subsumes both others. requirements.txt already carries pyyaml>=6.0, pytest>=9.0 and pyserial>=3.5, and this PR adds cryptography>=41.0, so installing from the file satisfies #90's pyyaml need and #88's cryptography need at once — and test_jobs_that_run_pytest_install_the_repository_requirements then prevents the parallel-list problem from coming back, which neither of the other two does.	Land this PR's line and drop the ci.yml hunks from #88 and #90 rather than resolving the conflict by picking one package list.

Verified clean, and worth recording because two of these are the parts a reviewer would most reasonably doubt:

• The link-line change is safe. CMakeLists.txt:130 is target_link_libraries(eboot_stage1 PUBLIC eboot_core), so dropping eboot_core from eboot_test_recovery's explicit list leaves it linked transitively. The comment's reasoning is shakier than the change — a duplicate archive on a link line is normally harmless rather than something "the linker reported" — but the edit itself is correct and the PUBLIC keyword is what makes it so.
• The new workflow-scanning test is satisfiable, not aspirational. test_jobs_that_run_pytest_install_the_repository_requirements asserts that every job matching pytest … tests/ also installs -r requirements.txt. Across all 16 files in .github/workflows/, ci.yml:69 (python3 -m pytest tests/ -v --tb=short) is the only match, and this PR fixes that job's installer; nightly.yml:84 already runs pip install -r requirements.txt. So the test passes at this head — which, given finding 1, no CI has demonstrated.
• _strip_comments() earns its place. This PR's own new YAML comment block at ci.yml:10-16 mentions pytest, tests/unit/test_sign_image.py and importorskip; without the comment-stripping it would register as a job that runs pytest and the new test would fail on its own diff. That is a considered detail.
• The baseline corroborates #90 independently. Both report 19 passed / 1 skipped for pytest tests/ on unmodified master, from different authors on different branches. #90's own 25-passed figure is 19 + its 6 new cases, and this PR's 36 is 19 + 14 + 3. The numbers across the two are consistent.

Architecture conformance

Conforms. §21 Tier 1 — Foundation, plus Infrastructure for the workflow and requirements.txt changes. core/ is the right home for both C edits per .ai/architect.md ("core/ shared boot logic") and both are platform-agnostic — no board or SoC specifics leak in, boards/, stage0/, hal/ and stage1/ are untouched, and the body is explicit that secure_boot.c, image_verify.c, ed25519_verify.c, recovery.c, keystore.c and jump_app.c are not touched either. No new include, link line or target_link_libraries entry points up a tier; the only linkage change removes an edge. §5.1's minimal-and-auditable TCB holds: three added lines in eboot_core, no new dependency. §8.1's "Crash/health information available to update logic" is the requirement both the boot-log fix and findings 3 and 4 sit under.

No proposal appended. The CI half of this PR is a fourth instance of a design gap already recorded — .ai/autoreview/proposals/2026-09.md, "The evidence policy is silent on checks that verify nothing" (§28, triggered by eAI#39, eAI#41, eBoot#81) — whose proposed §28.2 already covers it: "A test runner that finds zero tests must fail." An importorskip on an undeclared dependency is the same failure in a different runner, and this PR's test_requirements.py is a good concrete implementation of it. The existing text needs no amendment to cover the case, so I have left it alone rather than adding a near-duplicate entry.

Proposed changes

1. Get the workflow runs approved and executed (finding 1). Nothing else should be judged final until they report.
2. Replace the (void)port; suppressions with a rejection of an unsupported port, and mark the config field unimplemented (finding 2).
3. Add the erased-slot check before the write, and extend the mock to fail mid-entry so the partial-write path is covered (finding 3).
4. Follow up separately on eos_boot_log_append() returning a status (finding 4).
5. Coordinate the ci.yml line with #88 and #90 by keeping this PR's version (finding 5).

Items 2-5 are independent of each other. The commit-per-fix split means 2, 3 and 5 can each be dropped or landed on their own, which the body offers and which is the right structure for this change set.

Not checked

• Nothing was executed, and unusually, nothing was executed by CI either. No ctest, no pytest, no build, no warning diff. Reproducing needs the PR head checked out, which the run brief forbids — and here there is no CI result to fall back on, so this review rests entirely on reading the diff and the working tree, which for every file this PR touches is identical to master (13a7a02) — the local checkout sits on the unmerged branch fix/ed25519-low-order-keys at 8b88125, whose single commit touches only core/ed25519_verify.c and tests/unit/test_ed25519.c, neither of which this PR goes near. That is a materially weaker position than for the other seven PRs in this batch, and finding 1 exists because of it.
• Finding 3 is reasoned from NOR flash semantics, not observed. I did not build a partial-write mock, and I did not check whether eos_hal_flash_write()'s contract on any in-tree board guarantees all-or-nothing behaviour — if some HAL implementation does guarantee it, the finding narrows to the boards that do not. hal/ and boards/ were not read.
• The "4 warnings before → 1 after" claim was not verified, nor was "no new compiler warnings, diffed against a build of unmodified master". I confirmed the cause of the two -Wunused-parameter warnings by reading eos_hal.h:129,163, but not the counts, and not what the remaining deliberate warning is.
• test_sign_image.py's 14 cases were not inspected. The body describes them as pinning the v2 signed-header wire format. I did not read them, so I cannot confirm the count, that they all pass, or that they overlap usefully with the static assertions #87 adds to include/eos_image.h. Whether #87 and these 14 tests duplicate each other is an open question I did not answer.
• The regex-based workflow parsing was checked for satisfiability, not robustness. JOB_RE = ^ ([A-Za-z0-9_-]+):$ assumes job ids sit at exactly two-space indent and that everything after \njobs: is job content. It holds for the 16 current files as far as the one property I tested goes; I did not look for a workflow that would defeat it.
• mergeStateStatus: BLOCKED, mergeable: MERGEABLE, reviewDecision: REVIEW_REQUIRED. No merge attempted. tests/CMakeLists.txt collides with #84, #85, #88 and #89; .github/workflows/ci.yml collides with #88 and #90.

---

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