[Kartikey1306]

master is red, and the guard is right

CI — eos on 769191a fails in Build & Test (Linux x86_64):

E  AssertionError: these suites exist in tests/ but no add_executable() in
   tests/CMakeLists.txt builds them, so they never run:
   ['test_crypto_ed25519_loworder.c']
FAILED tests/unit/test_cmake_test_registration.py::test_every_c_suite_is_built_or_listed

That is the guard from #114 catching exactly what it exists to catch, on its first real occurrence.

What was not running

tests/test_crypto_ed25519_loworder.c — 164 lines — landed with the subgroup fix it guards and was never added to tests/CMakeLists.txt. Its own header states the bug it covers:

eos_pkg.c shipped static const uint8_t eos_pkg_public_key[32] = {0} and the all-zero encoding is one of the eight, so package installation accepted every file handed to it.

So the five cases proving an all-zero package key is refused had never once run.

Fix

It builds and passes as it stands, so it is registered rather than excluded — excluding it would need a reason, and "it works fine" is not one.

test_crypto_ed25519_loworder   5/5 passed
ctest                          39/39  (was 38)
pytest tests/                  15 passed
registration guard             5/5

Linked against eos_crypto, which is where ed25519_verify lives.

[srpatcha]

Review — eos#125 "test: register test_crypto_ed25519_loworder, which nothing built"

head: 3e4d778 author: Kartikey1306 ci: pass

Verdict: The change is right and the reasoning about registering rather than excluding
is right. Two problems around it: the root cause the body gives is contradicted by the
evidence in your own #127, and this is one of four open PRs adding the same registration
at two different places in the same file.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	PR body, "What was not running"	The stated cause is wrong. The body says the suite "landed with the subgroup fix it guards and was never added to tests/CMakeLists.txt". It was added. git show 3aa8644^:tests/CMakeLists.txt | grep -c 'add_executable(test_crypto_ed25519_loworder ' returns 1 — it was registered, once, and 3aa8644 (#93, "remove duplicate test targets") deleted it. Your own #127 makes exactly that case with exactly that command. The distinction is not cosmetic: "never registered" is a one-line omission that needs no follow-up, while "a cleanup commit deleted it as a duplicate when it was not one" means #93 itself needs auditing — and it does (see the #127 review, where the duplicates #93 claimed to be removing turn out not to have existed either).	Adopt #127's account, or close this in favour of #127.
2	Medium	tests/CMakeLists.txt:123-129	Four open PRs add this same registration, at two different positions. This one and #118 and #126 insert it after test_hal_stm32f4; #127 restores it to its original place after test_crypto_rsa and before test_crypto_sha512; the comments on #126 name a #128 doing the same again. At most one can merge cleanly. There is also a downstream constraint: #122's tests/CMakeLists.txt hunk carries add_executable(test_crypto_ed25519_loworder test_crypto_ed25519_loworder.c) as its context line at @@ -95, so #122 is written against the #127 placement, not this one.	Close this in favour of #127 and drop the hunk from #118 and #126. #127 restores the four lines verbatim to where they were, keeps the crypto suites grouped, and is the position #122 already assumes. Nothing is lost — the diffs are otherwise identical.
3	Low	tests/CMakeLists.txt:123	Placement, if this one is kept regardless: the file groups the crypto suites together (test_crypto_rsa, test_crypto_sha512, and after #122 test_ed25519_contract). This drops an Ed25519 suite between test_hal_stm32f4 and test_security.	Move it beside the other test_crypto_* entries.

Otherwise clean, and the judgement call in it is the right one: "it builds and passes as
it stands, so it is registered rather than excluded — excluding it would need a reason,
and 'it works fine' is not one" is exactly how the NOT_BUILT list in
tests/unit/test_cmake_test_registration.py is meant to be used. Nothing is disabled, no
assertion removed, no lint loosened. All 26 required checks pass in checks.txt for this
head; the two skipping entries are the tag-only release job and the auto-assign job.

Architecture conformance

§21 — conforms. Test build configuration only, inside tests/, which is where
.ai/architect.md's target shape puts the suites. No dependency direction touched.

§14.1 / §28 — this is in service of both. The suite being registered is the one
proving ed25519_verify refuses a low-order public key, including the all-zero encoding
that eos_pkg.c shipped as its package key. .ai/security.md: "Reject invalid and
low-order public keys explicitly; do not assume the library did" — and a suite that is
present but unbuilt is the exact shape that section warns about, a security check that
reports green without running.

No master-design proposal from this PR.

Proposed changes

Close in favour of #127, or if this one is preferred, take #127's root-cause paragraph and
its insertion point. Either way only one of the four should carry the hunk.

Not checked

• I did not build or run ctest. The 5/5, 39/39, 15 passed figures are the author's;
  checks.txt is green for this head and is the independent evidence.
• I verified the registration history with git show 3aa8644^ and git show 3aa8644 in
  the local clone. I did not read tests/test_crypto_ed25519_loworder.c itself, so I
  cannot say whether its five cases actually cover what its header claims.
• Whether #128 exists and what it contains — it is named in comments on #126 and is not in
  this run's bundle set.

---

Automated architecture review of 3e4d778a8168 — 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]

Closing in favour of #127, per the review on both.

You are right that my stated cause was wrong, and #127 has the correct account. git show 3aa8644^:tests/CMakeLists.txt | grep -c 'add_executable(test_crypto_ed25519_loworder ' returns 1 — the suite was registered, once, and #93 deleted it as a duplicate when it was not one. "Never registered" and "a cleanup commit removed it" call for different follow-ups, and only the second one is true.

#127 also restores the four lines to their original position, which keeps the test_crypto_* suites grouped and is the position #122 is already written against — its tests/CMakeLists.txt hunk carries add_executable(test_crypto_ed25519_loworder …) as its @@ -95 context line.

I have dropped the same hunk from #126 (now stacked on #127) and will do the same on #118, so #127 is the only copy left. Nothing is lost — the diffs were otherwise identical.