Repository navigation
fix(kernel): repair the broken priority-inheritance merge in sync.c - #94
Conversation
tests/CMakeLists.txt declared test_crypto_aes and test_crypto_sha512
twice each. A repeated add_executable target name is a hard CMake error,
not a warning:
CMake Error at tests/CMakeLists.txt:77 (add_executable):
add_executable cannot create target "test_crypto_aes" because
another target with the same name already exists.
CMake Error at tests/CMakeLists.txt:79 (add_test):
add_test given test NAME "test_crypto_aes" which already exists.
Configuration aborted, so the command the README and CLAUDE.md give for
building the test suite produced no build system at all and no test
could be built or run.
Kept the second copy of each pair, whose comments describe what the
suite actually covers, and gave test_crypto_ecc and test_crypto_rsa the
same one-line description for consistency. No target, source file or
link dependency changes.
sync.c and task.c did not compile. embeddedos-org#67 replaced the save/restore priority-inheritance scheme with one that recomputes an owner's effective priority from an invariant, and embeddedos-org#55 — written against the pre-embeddedos-org#67 tree — landed on top and put half of the old scheme back without removing the new call sites. The result referenced neither design completely: sync.c:66 error: 'mtx_t' has no member named 'original_prio' sync.c:149 error: implicit declaration of function 'task_valid' sync.c:150 error: 'g_blocked_on' undeclared sync.c:197 error: 'g_blocked_on' undeclared sync.c:206 error: implicit declaration of function 'pi_propagate' sync.c:225 error: 'g_blocked_on' undeclared task.c:576 error: redefinition of 'eos_task_set_current_internal' Restored the embeddedos-org#67 design, which the kernel documentation already describes and tests/test_priority_inherit.c already tests: effective = min( base_priority, priority of every task waiting on every mutex held ) Recomputing from that invariant is what makes the nested and chained cases come out right, and it subsumes what embeddedos-org#55 set out to fix: a waiter that times out is removed from the queue and pi_propagate() then drops the boost it was causing, because the boost is derived from the live waiter set rather than stashed in the mutex. embeddedos-org#55's other criterion is kept too — a full waiter table returns EOS_KERN_NO_MEMORY without boosting the owner. Also removed the duplicate eos_task_set_current_internal() that embeddedos-org#55 added to task.c, keeping embeddedos-org#67's version, which clears g_current on an out-of-range handle and marks the task RUNNING as the header documents. Folded the surviving declaration's two doc comments together in kernel_internal.h.
#94 repairs the same spliced priority-inheritance code as this PR, and converged on the same design independently — g_blocked_on, task_valid, pi_recompute, pi_propagate, recomputing from the invariant rather than saving and restoring a value around each lock. The two implementations of sync.c differ by nothing except these two comments, which explain the consequence rather than only the action: "...it would sleep forever." -> "...it would sleep forever. Refusing also avoids boosting the owner on behalf of a caller that never waits." "Timeout — leave the queue and drop the boost we were causing." -> "...so a waiter that gave up no longer keeps the owner running elevated." Both name the priority-inheritance effect of the branch, which is the part a reader is likely to get wrong when editing it later. Comment-only; sync.c is otherwise byte-identical to #94's. 28/28 tests passed, 0 build errors Co-authored-by: prakhar7017 <prakhar7017@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
srpatcha
left a comment
There was a problem hiding this comment.
Correct fix, and you reached the right design independently.
I diffed your sync.c against #82's, which repairs the same splice:
$ git diff pr82 pr94 -- kernel/src/sync.c
Eight changed lines, all of them comments. The code is byte-identical:
g_blocked_on, task_valid, pi_recompute, pi_propagate, recomputing the
effective priority from the invariant rather than saving and restoring a value
around each lock. Two people arriving at the same implementation from the same
broken starting point is a good sign the design is the right one.
Your comments are better in two places, and I have folded both into #82 with
attribution:
"...it would sleep forever."
-> "...it would sleep forever. Refusing also avoids boosting the owner
on behalf of a caller that never waits."
"Timeout — leave the queue and drop the boost we were causing."
-> "...so a waiter that gave up no longer keeps the owner running elevated."
Both name the priority-inheritance consequence of the branch, which is the part
a later editor is most likely to get wrong. Same for the per-test crypto
comments this carries from #93 — also adopted.
Why the splice happened, since it will otherwise recur
#55 and #67 implemented priority inheritance with incompatible designs — one
keeping original_prio on the mutex, the other g_blocked_on with
pi_propagate(). Each was green on its own branch. Squashing both onto a base
that had moved spliced half of each together, producing a file referencing
fields and functions that no version ever defined at the same time. Neither PR
was wrong; the merge was never compiled.
The repository has no required status check:
$ gh api repos/embeddedos-org/eos/branches/master/protection
{"checks": null, "reviews": 1, "sigs": false}
Nothing builds the merge result before it becomes master, which is why four
individually-green PRs combined into something that had never been compiled in
the form it landed in. Tracked as #92.
Approving. #82 is broader — it also repairs the duplicate
eos_task_set_current_internal definition and the duplicate crypto test targets
— so if that lands first this becomes empty and can close. Either way your two
comment improvements are now in the tree.
* fix: repair master — four merges landed code that does not compile `master` does not configure, and once it does it does not build, and once it builds one suite fails. Each break is the same failure: a PR verified green on its own branch, squash-merged onto a base that had moved, and nothing re-verified the result. 1. tests/CMakeLists.txt registered test_crypto_aes and test_crypto_sha512 twice (#59 landing beside the block that already had them), so CMake refused to configure at all: add_test given test NAME "test_crypto_aes" which already exists 2. eos_task_set_current_internal was defined twice in task.c and declared twice in kernel_internal.h, with two different behaviours — one clears g_current and marks the task RUNNING, the other silently returns. Kept the first, which is what the header documents ("Handles >= EOS_MAX_TASKS clear it") and what the mutex tests rely on; folded the second declaration's comment into the surviving one, since it records why the function exists. 3. sync.c would not compile: g_blocked_on, pi_propagate and task_valid were used and defined nowhere, alongside a reference to a struct member original_prio that mtx_t does not have. #67 and #55 implement incompatible designs for the same feature. #67 recomputes effective priority from effective = min(base_priority, priority of every waiter on every mutex the task holds) on every change; #55 saves and restores a value around each lock. Merging #55 after #67 deleted #67's implementation while leaving its call sites, so neither design was left intact. Restored #67's. It is the stronger of the two — save/restore is wrong when a task holds two mutexes and releases one, and wrong again when a task is boosted while already holding a mutex — and it subsumes what #55 fixes: recomputing on timeout is one of the cases it recomputes on, and it already rejects a full waiter table rather than blocking a task that was never enqueued. 4. #74's tests were merged without its parser change. Six assertions in test_config failed because a "- name:" item arriving while section was SEC_PKG_DEPS fell into the dependency branch, so the package after a deps list was swallowed as a dependency of the previous one. Restored the SEC_PKG_DEPS arm of the outer condition, nested the dependency handling as the else of the name check, and added the deps transition under SEC_PKG_BUILD. before cmake: configure fails after build clean, ctest 28/28 Also re-checked the package-overflow guard that arrived via #61/#65, since it landed in a different shape than #78 proposed: AddressSanitizer is clean on a 129-package config that previously overflowed, and the 129th entry is refused with a message rather than silently dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(core): make eos cross-compile for Cortex-M4 again eos does not build for the target it exists to build for. From a clean checkout of master, with the toolchain the CI job installs: $ cmake -B barm -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-cortex-m4.cmake $ cmake --build barm core/src/log.c:113:9: error: implicit declaration of function 'clock_gettime' core/src/log.c:113:23: error: 'CLOCK_MONOTONIC' undeclared get_timestamp_ms() had two branches, _WIN32 and "else, assume POSIX". Bare- metal newlib is neither: arm-none-eabi declares neither clock_gettime nor CLOCK_MONOTONIC, so the POSIX branch cannot compile for any Cortex-M target. The macro is the precise test — where it is absent, so is the function. There is a timebase on a target, eos_get_tick_ms() in the HAL, but core/ sits below hal/ and must not depend upward, so log entries carry a zero timestamp on a freestanding build until a port supplies one. Ordering within the ring buffer is unaffected; only absolute time is missing. Also fixed the format specifier one line down. uint32_t is `unsigned int` on x86-64 and `unsigned long` on 32-bit ARM, so the literal %u was wrong on exactly the target this build is for. It is PRIu32 now, which is right on both. Why CI did not catch it. The ARM job points CMAKE_TOOLCHAIN_FILE at cmake/arm-cortex-m4.cmake; the file is at toolchains/. That fails loudly -- "Could not find toolchain file" -- so the job has been red rather than silently passing, but it dies at configure and never reaches the compile step where this bug lives. The path is not new breakage: #75 fixed it, #79 and #51 carried the fix, and #59 reverted it. That is the same stale-base merge pattern as the other three repairs on this branch. before ARM: configure fails on a missing toolchain file; with the real path, 2 compile errors in core/src/log.c after ARM: 70/70 targets built, artifacts report `architecture: armv7e-m`; host: 28/28 ctest, unchanged Kernel and services together measure 15.8 KB flash and 26 KB RAM on Cortex-M4, which is the first real footprint number this repo has produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): describe what each crypto test covers Deduplicating test_crypto_aes and test_crypto_sha512 left the grouped comment that had been sitting above the block: # --- test_crypto_aes / _ecc / _rsa / _sha512: were sitting in tests/ unused --- That records how the files got here, which stops being interesting the moment they are wired up. #93 fixes the same two duplicates and keeps the per-test descriptions instead: # --- test_crypto_aes: AES-128/192/256 ECB + CBC, NIST FIPS-197 / SP 800-38A --- # --- test_crypto_ecc: ECC point arithmetic and ECDSA --- # --- test_crypto_rsa: RSA modular exponentiation and verify --- # --- test_crypto_sha512: SHA-512 against NIST vectors --- Naming the standard each vector set comes from is worth more to the next reader than a note about the file's history. Adopting that here so the better wording survives the conflict between the two PRs. Comment-only; no target, source or link line changes. 28/28 tests passed, 0 build errors Co-authored-by: prakhar7017 <prakhar7017@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(sync): say why each priority-inheritance branch acts as it does #94 repairs the same spliced priority-inheritance code as this PR, and converged on the same design independently — g_blocked_on, task_valid, pi_recompute, pi_propagate, recomputing from the invariant rather than saving and restoring a value around each lock. The two implementations of sync.c differ by nothing except these two comments, which explain the consequence rather than only the action: "...it would sleep forever." -> "...it would sleep forever. Refusing also avoids boosting the owner on behalf of a caller that never waits." "Timeout — leave the queue and drop the boost we were causing." -> "...so a waiter that gave up no longer keeps the owner running elevated." Both name the priority-inheritance effect of the branch, which is the part a reader is likely to get wrong when editing it later. Comment-only; sync.c is otherwise byte-identical to #94's. 28/28 tests passed, 0 build errors Co-authored-by: prakhar7017 <prakhar7017@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: prakhar7017 <prakhar7017@users.noreply.github.com>
|
Reviewing this against 1. Against this PR's own merge base ( The PR removes all four. The first two are right. The other two are unique
# --- test_crypto_failclosed: unimplemented RSA/ECC stubs refuse to run ---
# Must NOT define EOS_ALLOW_STUB_CRYPTO; this is the production fail-closed path.Dropping the target does not fail anything — the file just stops being built — 2. The actual change is two trailing blank lines; the other ~1148 changed lines are 3. It does not merge as it stands. Also worth knowing before you rebase: @srpatcha's comment on #92 reports the None of this is a criticism of the analysis — the CHANGELOG entry describing the |
|
Update: the failure mode I flagged above has now happened, via #93. #93 merged and removed That is the same mechanism I described on this PR: a "remove duplicates" diff So the concern is no longer hypothetical — please rebase onto current 2 means a genuine duplicate, 1 means the diff is deleting a live target. |
|
|
||
| memset(&g_tasks[slot], 0, sizeof(eos_task_t)); | ||
| g_tasks[slot].id = (uint8_t)slot; | ||
| g_tasks[slot].name = name; |
| g_timers[i].period_ticks = period_ms; /* 1 tick = 1 ms */ | ||
| g_timers[i].next_tick = 0; | ||
| g_timers[i].callback = callback; | ||
| g_timers[i].ctx = ctx; |
srpatcha
left a comment
There was a problem hiding this comment.
Review — eos#94 "fix(kernel): repair the broken priority-inheritance merge in sync.c"
head: 561a0d3 author: prakhar7017 ci: fail
Verdict: The kernel repair this PR was opened for has already landed on master
by another route, so what is left in the diff is two comment improvements plus two
unintended regressions — a CRLF rewrite of task.c and the silent deletion of eight
CHANGELOG.md entries, one of which records the eos_pkg signature-verification fix
from #98. The merge is now conflict-free, so nothing would stop either from landing.
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | High | CHANGELOG.md:6-20 |
The diff removes eight [Unreleased] entries that were added to master after this branch's base, including the eos_pkg trust-anchor fix (eos_pkg_public_key[32] = {0} accepting every package, #98/#99), services/pkg is compiled, ed25519_public_key_is_usable(), the eos_queue_send/_receive waiter-table fix, all three eos_dt_* out-of-bounds-read fixes, and the entire ### Added section (tests/test_devicetree.c, tests/fuzz/ wiring). The merge is clean, so this loss is silent. |
Rebase onto origin/master and re-apply only the two lines this PR actually adds; keep every entry already present upstream. |
| 2 | Medium | kernel/src/task.c (whole file) |
task.c is rewritten LF → CRLF. git diff --numstat reports 574 576; --ignore-cr-at-eol reports 0 2. The only real edit is the removal of two trailing blank lines. Merging converts master's task.c to CRLF and will conflict with any other change to the file. |
Rewrite the file with LF (git checkout origin/master -- kernel/src/task.c, then re-apply the blank-line trim if it is wanted), and confirm with git diff --numstat before pushing. |
| 3 | Medium | PR body, "Changes" and "Testing" | The body describes work the diff no longer contains — kernel/include/eos/kernel_internal.h is not in files.txt, and the sync.c restoration it details is already on master. The new CHANGELOG entry likewise claims the tests/CMakeLists.txt duplicate-target fix and the sync.c/task.c compile fix, neither of which is in this diff. The "Same compile on master — FAIL — the 7 errors quoted above" line no longer holds. |
Rewrite the body and the CHANGELOG entry to describe only what the rebased diff changes. |
| 4 | Low | CI | Four required checks are red at this head: Build & Test (Linux x86_64), Cross-compile ARM64 kernel, Run Python tests, Full-stack integration summary. The body attributes the remaining breakage to core/src/log.c, but that explanation predates the rebase and is no longer evidence for these four. |
Re-derive the failures after the rebase and either fix or attribute them individually. |
Evidence for 1 and 2, run against origin/master at feee272:
$ git merge --no-commit --no-ff pr94
Automatic merge went well; stopped before committing as requested
$ grep -c "trust anchor" CHANGELOG.md # in the merged tree
0
$ for s in eos_dt_parse tests/fuzz test_devicetree services/pkg eos_queue_send; do
printf '%-18s %s\n' "$s" "$(grep -c "$s" CHANGELOG.md)"; done
eos_dt_parse 0
tests/fuzz 0
test_devicetree 0
services/pkg 0
eos_queue_send 0
$ file kernel/src/task.c
kernel/src/task.c: C source, ..., with CRLF line terminators
The stated problem is already fixed upstream:
$ git show origin/master:kernel/src/sync.c | grep -c original_prio
0
$ git show origin/master:kernel/src/sync.c | grep -c pi_propagate
7
$ git show origin/master:kernel/src/task.c | grep -c eos_task_set_current_internal
1
$ gcc -std=c11 -Wall -Wextra -Werror -O0 -g -Ikernel/include -Iinclude -o build_e2e \
tests/test_e2e.c kernel/src/task.c kernel/src/sync.c kernel/src/ipc.c kernel/src/mem/heap.c
# exit 0, on origin/master alone, without this PR
The two sync.c comment hunks (sync.c:171-176, sync.c:197-203) are an improvement
on what is there and are worth keeping.
Architecture conformance
Conforms. Master design §5.1 and §6: the change is confined to kernel/src/, adds no
include or link edge, and introduces no dependency on a higher tier — sync.c and
task.c reference only kernel-internal symbols. Tier 1 (§21) is the correct home for
kernel synchronisation primitives. §6.1's requirement that "interrupt, timer and
synchronization semantics must be documented and testable" is better served by the
restored recompute-from-invariant design than by the original_prio variant, and
master already carries it.
Finding 1 does touch §28 (Status, Evidence and Claims Policy): deleting the changelog
entry for the #98 signature-verification fix removes the only consumer-visible record
that the fix shipped, while the code stays. §28 requires evidence to match state; this
would leave an Implemented security fix undocumented.
Severity note per .ai/reviewer.md: finding 1 is High rather than Critical. No code,
test target or coverage is lost, and the deletion is recoverable from git history — the
loss is the release-notes record. It is not rounded down below High because it is
silent, it lands without conflict, and the entry it erases is for a signature bypass.
Proposed changes
Smallest sequence that keeps the useful part:
git fetch origin && git rebase origin/mastergit checkout origin/master -- CHANGELOG.md kernel/src/task.c— both files are
fully superseded upstream; this PR needs no change in either.- Keep only the two
sync.ccomment hunks.git diff --numstat origin/mastershould
then read2 2 kernel/src/sync.cand nothing else. - Replace the PR body with a one-paragraph description of the comment change.
If the two trailing blank lines in task.c are genuinely worth removing, do it as a
separate LF-preserving commit; do not carry it inside this PR.
Not checked
- I did not run the failing CI jobs, and I have no access to their logs. The claim that
the four red checks are unrelated to this diff is neither confirmed nor refuted here;
I only established that the compile the PR body cites now passes onmasteralone. tests/test_priority_inheritandtests/test_kernelwere not executed. The PR body's
PASS table for them is untested by me, and is in any case a claim about code that is
already upstream.- I verified
kernel/src/task.cis byte-identical toorigin/mastermodulo line
endings and two blank lines. I did not audit the remaining 574 CRLF-converted lines
for semantic change beyond that--ignore-cr-at-eoldiff. - The two earlier review comments on this PR (
tests/CMakeLists.txtdeleting
test_crcandtest_crypto_failclosed; the merge conflict) are resolved at this
head —tests/CMakeLists.txtis no longer in the diff and the merge is clean. I did
not re-verify the target counts they cite.
Automated architecture review of 561a0d3face2 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.
srpatcha
left a comment
There was a problem hiding this comment.
Review — eos#94 "fix(kernel): repair the broken priority-inheritance merge in sync.c"
head: 561a0d3 author: prakhar7017 ci: fail
Verdict: The kernel repair this PR was opened for has already landed on master
by another route, so what is left in the diff is two comment improvements plus two
unintended regressions — a CRLF rewrite of task.c and the silent deletion of eight
CHANGELOG.md entries, one of which records the eos_pkg signature-verification fix
from #98. The merge is now conflict-free, so nothing would stop either from landing.
Findings
| # | Severity | File:line | Finding | Recommended fix |
|---|---|---|---|---|
| 1 | High | CHANGELOG.md:6-20 |
The diff removes eight [Unreleased] entries that were added to master after this branch's base, including the eos_pkg trust-anchor fix (eos_pkg_public_key[32] = {0} accepting every package, #98/#99), services/pkg is compiled, ed25519_public_key_is_usable(), the eos_queue_send/_receive waiter-table fix, all three eos_dt_* out-of-bounds-read fixes, and the entire ### Added section (tests/test_devicetree.c, tests/fuzz/ wiring). The merge is clean, so this loss is silent. |
Rebase onto origin/master and re-apply only the two lines this PR actually adds; keep every entry already present upstream. |
| 2 | Medium | kernel/src/task.c (whole file) |
task.c is rewritten LF → CRLF. git diff --numstat reports 574 576; --ignore-cr-at-eol reports 0 2. The only real edit is the removal of two trailing blank lines. Merging converts master's task.c to CRLF and will conflict with any other change to the file. |
Rewrite the file with LF (git checkout origin/master -- kernel/src/task.c, then re-apply the blank-line trim if it is wanted), and confirm with git diff --numstat before pushing. |
| 3 | Medium | PR body, "Changes" and "Testing" | The body describes work the diff no longer contains — kernel/include/eos/kernel_internal.h is not in files.txt, and the sync.c restoration it details is already on master. The new CHANGELOG entry likewise claims the tests/CMakeLists.txt duplicate-target fix and the sync.c/task.c compile fix, neither of which is in this diff. The "Same compile on master — FAIL — the 7 errors quoted above" line no longer holds. |
Rewrite the body and the CHANGELOG entry to describe only what the rebased diff changes. |
| 4 | Low | CI | Four required checks are red at this head: Build & Test (Linux x86_64), Cross-compile ARM64 kernel, Run Python tests, Full-stack integration summary. The body attributes the remaining breakage to core/src/log.c, but that explanation predates the rebase and is no longer evidence for these four. |
Re-derive the failures after the rebase and either fix or attribute them individually. |
Evidence for 1 and 2, run against origin/master at feee272:
$ git merge --no-commit --no-ff pr94
Automatic merge went well; stopped before committing as requested
$ grep -c "trust anchor" CHANGELOG.md # in the merged tree
0
$ for s in eos_dt_parse tests/fuzz test_devicetree services/pkg eos_queue_send; do
printf '%-18s %s\n' "$s" "$(grep -c "$s" CHANGELOG.md)"; done
eos_dt_parse 0
tests/fuzz 0
test_devicetree 0
services/pkg 0
eos_queue_send 0
$ file kernel/src/task.c
kernel/src/task.c: C source, ..., with CRLF line terminators
The stated problem is already fixed upstream:
$ git show origin/master:kernel/src/sync.c | grep -c original_prio
0
$ git show origin/master:kernel/src/sync.c | grep -c pi_propagate
7
$ git show origin/master:kernel/src/task.c | grep -c eos_task_set_current_internal
1
$ gcc -std=c11 -Wall -Wextra -Werror -O0 -g -Ikernel/include -Iinclude -o build_e2e \
tests/test_e2e.c kernel/src/task.c kernel/src/sync.c kernel/src/ipc.c kernel/src/mem/heap.c
# exit 0, on origin/master alone, without this PR
The two sync.c comment hunks (sync.c:171-176, sync.c:197-203) are an improvement
on what is there and are worth keeping.
Architecture conformance
Conforms. Master design §5.1 and §6: the change is confined to kernel/src/, adds no
include or link edge, and introduces no dependency on a higher tier — sync.c and
task.c reference only kernel-internal symbols. Tier 1 (§21) is the correct home for
kernel synchronisation primitives. §6.1's requirement that "interrupt, timer and
synchronization semantics must be documented and testable" is better served by the
restored recompute-from-invariant design than by the original_prio variant, and
master already carries it.
Finding 1 does touch §28 (Status, Evidence and Claims Policy): deleting the changelog
entry for the #98 signature-verification fix removes the only consumer-visible record
that the fix shipped, while the code stays. §28 requires evidence to match state; this
would leave an Implemented security fix undocumented.
Severity note per .ai/reviewer.md: finding 1 is High rather than Critical. No code,
test target or coverage is lost, and the deletion is recoverable from git history — the
loss is the release-notes record. It is not rounded down below High because it is
silent, it lands without conflict, and the entry it erases is for a signature bypass.
Proposed changes
Smallest sequence that keeps the useful part:
git fetch origin && git rebase origin/mastergit checkout origin/master -- CHANGELOG.md kernel/src/task.c— both files are
fully superseded upstream; this PR needs no change in either.- Keep only the two
sync.ccomment hunks.git diff --numstat origin/mastershould
then read2 2 kernel/src/sync.cand nothing else. - Replace the PR body with a one-paragraph description of the comment change.
If the two trailing blank lines in task.c are genuinely worth removing, do it as a
separate LF-preserving commit; do not carry it inside this PR.
Not checked
- I did not run the failing CI jobs, and I have no access to their logs. The claim that
the four red checks are unrelated to this diff is neither confirmed nor refuted here;
I only established that the compile the PR body cites now passes onmasteralone. tests/test_priority_inheritandtests/test_kernelwere not executed. The PR body's
PASS table for them is untested by me, and is in any case a claim about code that is
already upstream.- I verified
kernel/src/task.cis byte-identical toorigin/mastermodulo line
endings and two blank lines. I did not audit the remaining 574 CRLF-converted lines
for semantic change beyond that--ignore-cr-at-eoldiff. - The two earlier review comments on this PR (
tests/CMakeLists.txtdeleting
test_crcandtest_crypto_failclosed; the merge conflict) are resolved at this
head —tests/CMakeLists.txtis no longer in the diff and the merge is clean. I did
not re-verify the target counts they cite.
Automated architecture review of 561a0d3face2 — 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.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red.
|
None of the four red checks here are caused by this PR. They are two separate
So there is nothing to change in this PR. Once #127 lands, rebasing will clear |
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
The pin in the previous commit stops an eBoot or ebuild master that does not
compile from turning every eos pull request red. It also stops that break
being visible from here at all, which on its own is the worse half of the
trade: the next upstream breakage would be silent instead of loud, and the
pin would sit at a172a6d indefinitely because nothing would ever ask.
This is the other half. `upstream-drift` builds eBoot at master and compares
both pins against their master, so an upstream break still surfaces -- on a
run that blocks no pull request and names the repository responsible.
Two deliberate choices, both about not repeating defects this repo has been
filing against:
It never runs on `pull_request`. A PR must not be red for a break it did
not cause; that is the entire point of the pin.
It is not `continue-on-error`. A job that reports success over a failure is
the fail-open shape we keep finding in aggregating gates. This one is free
to fail honestly precisely because it gates nothing -- schedule and
workflow_dispatch only.
Failing is reserved for "upstream does not build". A pin merely behind a
healthy master is a warning, because that is actionable rather than broken.
The failure message is scoped to the build step's own outcome, so a checkout
or apt failure cannot report itself as evidence about eBoot's master. The
summary states that eBoot is built and ebuild is only compared, rather than
implying both were checked.
Verified by execution, not by reading:
eBoot @ a172a6d (the pin) host build rc=0, 0 errors
eBoot @ 22d8f8b (master) host build rc=2, "no member named 'reserved'
in 'eos_image_header_t'" at eos_image.h:135
and :142 -- the same defect CI reports, so the
pin is demonstrably the difference
drift logic, run against the real repositories:
pin 6 behind healthy master -> "behind by 6" warning, no failure
pin absent from master -> "not an ancestor" warning, exit 0 under
set -e (the first draft printed "pinned
pin not found on master commit(s)")
pin equal to master -> no warning
eos on this branch: cmake build rc=0, ctest 39/39, pytest 15/15,
test_crypto_ed25519_loworder now registered and passing.
When eBoot embeddedos-org#94 lands, bump EBOOT_COMMIT to that merge; this job is what will
say whether it holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pin in the previous commit stops an eBoot or ebuild master that does not compile from turning every eos pull request red. It also stops that break being visible from here at all, which on its own is the worse half of the trade: the next upstream breakage would be silent instead of loud, and the pin would sit at a172a6d indefinitely because nothing would ever ask -- while quietly dropping the six eBoot commits that follow it. `.github/workflows/upstream-drift.yml` is the other half. It builds eBoot at master and compares both pins against their master, so an upstream break still surfaces, on a run that blocks no pull request and names the repository responsible. Design choices, each one avoiding a defect this repo has been filing against: Its own file, schedule + workflow_dispatch only. The first draft was a job inside eos-simulation.yml guarded by `if: github.event_name == 'schedule'`. That works, but the run showed it on the PR as a SKIPPED check -- and a required-check gate that treats any non-success as a failure, which is the design in embeddedos-org#121, would trip on exactly that. Moving it out removes the check from PR runs entirely rather than relying on every future gate to exempt it. Not `continue-on-error`. A job reporting success over a failure is the fail-open shape we keep finding in aggregating gates. This one may fail honestly precisely because it gates nothing. Failing is reserved for "upstream does not build". A pin merely behind a healthy master is a warning: actionable, not broken. The pins are read out of eos-simulation.yml, not copied. A second copy of a SHA is a second thing to forget. The read demands a 40-character hex value and fails closed if it cannot find one, so a renamed or reformatted key stops the job instead of silently letting it check a pin of "". The failure message is scoped to the build step's own outcome, so a checkout, apt or pin-read failure cannot report itself as evidence about eBoot's master. The summary says eBoot is built and ebuild is only compared, rather than implying both were checked. Verified by execution, not by reading: eBoot @ a172a6d (the pin) host build rc=0, 0 errors eBoot @ 22d8f8b (master) host build rc=2, "no member named 'reserved' in 'eos_image_header_t'" at eos_image.h:135 and :142 -- the same defect CI reports, so the pin is demonstrably the difference pin extraction, run against the real file and two mutations of it: real workflow -> both SHAs read correctly, and the eBoot one resolves to embeddedos-org#86's merge key renamed to EBOOT_SHA -> fails closed, rc=1 value unquoted and shortened -> fails closed, rc=1 drift logic, run against the real repositories: pin 6 behind healthy master -> "behind by 6" warning, no failure pin absent from master -> "not an ancestor" warning, exit 0 under set -e (the first draft printed "pinned pin not found on master commit(s)") pin equal to master -> no warning eos on this branch: cmake build rc=0, ctest 39/39, pytest 15/15 with the new workflow file present, test_crypto_ed25519_loworder registered and passing. When eBoot embeddedos-org#94 lands, bump EBOOT_COMMIT to that merge; this job is what will say whether it holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * chore(tests): drop the unused assert.h include from test_net.c Retitles and reduces this PR to what it still does, per the review. Both repairs it was opened for have landed on master by other routes: net/src/net_posix.c is byte-identical to origin/master, and master's default configuration builds clean. Against current master the branch's only real content was this one line -- and merging it as it stood would have *reverted* 646 lines that landed since, including #120's pkg trust anchor, tests/test_pkg_trust_anchor.c and tests/unit/test_cmake_test_registration.py. Reset to master and re-applied the one surviving change rather than leave a stale branch that still looks mergeable. The include is genuinely unused: tests/test_net.c has zero assert() calls and uses its own CHECK() macro at 22 call sites. Worth correcting the record, per finding 2: #124 added it one commit ago as "the missing assert.h include that broke master", and the file had no assert() at #124's parent either. What fixed that link error was the assert() -> CHECK() conversion already on master; the include was never load-bearing, so removing it changes nothing except the next reader's understanding. Stacked on #127, which restores the test_crypto_ed25519_loworder registration master is currently red on. Verified: pytest tests/ 15 passed ctest 39/39 PASS Refs #116, #124 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review.
…135) * fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. * ci: make the pin visible when it goes stale, not silently permanent The pin in the previous commit stops an eBoot or ebuild master that does not compile from turning every eos pull request red. It also stops that break being visible from here at all, which on its own is the worse half of the trade: the next upstream breakage would be silent instead of loud, and the pin would sit at a172a6d indefinitely because nothing would ever ask -- while quietly dropping the six eBoot commits that follow it. `.github/workflows/upstream-drift.yml` is the other half. It builds eBoot at master and compares both pins against their master, so an upstream break still surfaces, on a run that blocks no pull request and names the repository responsible. Design choices, each one avoiding a defect this repo has been filing against: Its own file, schedule + workflow_dispatch only. The first draft was a job inside eos-simulation.yml guarded by `if: github.event_name == 'schedule'`. That works, but the run showed it on the PR as a SKIPPED check -- and a required-check gate that treats any non-success as a failure, which is the design in #121, would trip on exactly that. Moving it out removes the check from PR runs entirely rather than relying on every future gate to exempt it. Not `continue-on-error`. A job reporting success over a failure is the fail-open shape we keep finding in aggregating gates. This one may fail honestly precisely because it gates nothing. Failing is reserved for "upstream does not build". A pin merely behind a healthy master is a warning: actionable, not broken. The pins are read out of eos-simulation.yml, not copied. A second copy of a SHA is a second thing to forget. The read demands a 40-character hex value and fails closed if it cannot find one, so a renamed or reformatted key stops the job instead of silently letting it check a pin of "". The failure message is scoped to the build step's own outcome, so a checkout, apt or pin-read failure cannot report itself as evidence about eBoot's master. The summary says eBoot is built and ebuild is only compared, rather than implying both were checked. Verified by execution, not by reading: eBoot @ a172a6d (the pin) host build rc=0, 0 errors eBoot @ 22d8f8b (master) host build rc=2, "no member named 'reserved' in 'eos_image_header_t'" at eos_image.h:135 and :142 -- the same defect CI reports, so the pin is demonstrably the difference pin extraction, run against the real file and two mutations of it: real workflow -> both SHAs read correctly, and the eBoot one resolves to #86's merge key renamed to EBOOT_SHA -> fails closed, rc=1 value unquoted and shortened -> fails closed, rc=1 drift logic, run against the real repositories: pin 6 behind healthy master -> "behind by 6" warning, no failure pin absent from master -> "not an ancestor" warning, exit 0 under set -e (the first draft printed "pinned pin not found on master commit(s)") pin equal to master -> no warning eos on this branch: cmake build rc=0, ctest 39/39, pytest 15/15 with the new workflow file present, test_crypto_ed25519_loworder registered and passing. When eBoot #94 lands, bump EBOOT_COMMIT to that merge; this job is what will say whether it holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * chore: untrack the sanitizer build trees committed in #110 bsan/ and bsan2/ are CMake build directories. They reached master in 8a2d835 (#110) — 1441 files, 12.2 MB, four compiled object files, and two CMakeCache.txt holding absolute paths from the machine that ran the sanitizer build. Nothing in the repository refers to either directory. They are mine, and they are the same mistake as the .coverage database that #75 carried to master and #79 had to remove: `git add -A` in a tree with a build directory whose name no ignore pattern matched. .gitignore already lists build/, _build/, build-*/, build_coverage/, build_qemu_arm64/ and build_sim/ — every one of them added after being committed once. Widened to cover bsan*/ so this name cannot come back. Removed with `git rm -r --cached`, so anyone with a local sanitizer build keeps it; it is simply no longer tracked. Contents only — no source, build or CI file is touched, and the tree builds and tests exactly as before: -DEOS_BUILD_TESTS=ON -DEOS_PRODUCT=vbox_test + ctest -> 34/34 passed * chore: collapse the redundant bsan ignore patterns, and say what untracking does not do Answers the review on #117. Finding 1 (Low) -- bsan*/ already matches bsan/ and bsan2/, so those two lines were dead the moment the third was written. This file's stated problem is that it accumulates one redundant entry per incident; adding two more while fixing that is the wrong direction. Verified the single pattern still covers all three shapes: bsan/, bsan2/ and bsan99/ are each ignored with only bsan*/ present. Finding 2 (Low) -- the body read as though the trees were gone. They are only untracked going forward: the 12.2 MB and the absolute paths stay in history at 8a2d835 permanently, and every clone still downloads them. Said so in the PR body rather than leaving the next reader to work it out. No history rewrite is proposed -- the reviewer scanned all 1441 blobs for credential-shaped content and found none, the only disclosure being a public GitHub handle, so a rewrite would cost every consumer a re-clone for no security benefit. Verified: pytest tests/ 15 passed bsan/, bsan2/, bsan99/ with only bsan*/ all three ignored git status --short clean Refs #117 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review.
* fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review.
…rd it (#114) * fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * test: stop building test_net_mock, which covers nothing, and say what the guard checks Reduces this PR to what it still contributes, and answers the review. The four registrations it opened with have landed on master by other routes -- test_mpu_validate, test_e2e, test_hal_stm32f4 and the registration guard are all there. Merging the branch as it stood would have reverted 505 lines that landed since, including #120's pkg trust anchor. Reset onto #127 and kept only what is still true. Finding 1 (Medium) -- test_net_mock was registered with no target_link_libraries and includes no EmbeddedOS header. Its mock_net_* functions are called directly rather than standing in for eos_net_*, so no change to net/ can make it fail: 614 lines asserting a mock against itself. Registering it added a green ctest entry covering nothing, which is precisely the failure mode the guard in this same commit exists to surface -- and the guard's own NOT_BUILT policy is where it belongs. Listed there with the reason, including what would make it worth building: driving the real eos_net_* API against the mock transport, which is what its docblock already claims it does. Finding 2 (Low) -- documented the guard's limit where the policy is defined. It checks that every suite is built and registered with ctest. It does not check that a suite reaches the code it is named after, and a file that links nothing satisfies it completely. Worth stating next to NOT_BUILT, because that is the list someone reaches for when the guard fires. Findings 3 and 4 are moot at this head: the test_mpu_validate comment and test_net_mock's vestigial assert.h include both belonged to hunks that are either upstream or removed here. Verified: pytest tests/ 15 passed ctest 38/38 PASS (test_net_mock is no longer among them; the count is unchanged from master because #127, which this is stacked on, adds test_crypto_ed25519_loworder -- a suite that does exercise real code.) Refs #114 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review.
* fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * fix(pkg): refuse a source download that cannot be verified eos_fetch_source() compared the archive against its expected digest only when one was supplied: if (expected_hash && expected_hash[0]) { ...compare... } so calling it without a digest downloaded the archive and extracted it having compared it against nothing -- and returned EOS_OK, which is what a verified fetch also returns. The caller cannot tell the two apart, and the argument is an ordinary optional parameter of a public function declared in pkg/include/eos/package.h. Refuse instead, before the network is touched. Git URLs stay exempt: a clone carries its own object hashes. ebuild made the same change to PackageFetcher.fetch() for the same reason (embeddedos-org/ebuild#67) -- omitting one field bought an unverified download there too. tests/test_pkg_fetch.c is new; the function had no tests. Every case is offline by construction, because returning before the network is touched is the property under test: each archive form this fetcher extracts is refused without a digest, an empty digest is refused rather than treated as absent, an unsafe URL still fails as unsafe rather than as a checksum error, and an empty URL is still the no-op it was. 39/39. Against the unfixed fetcher the suite fails on its first assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pkg): honour the digest on the git path too, and anchor is_git_url Answers the review on #115. The tarball half of this PR was correct; the identical defect was still live one branch over. Finding 1 (High) -- the guard read `if (is_tarball(url) && ...)`, so a git URL passed it with a digest and without one, and fetch_git() then never looked at expected_hash. A caller that pinned a revision got EOS_OK and the content with nothing compared -- verbatim the property this PR's own body names as the defect: "the caller cannot tell the two apart, because both return EOS_OK." fetch_git() now takes the pin and compares it against the commit the clone actually landed on (`git -C <dest> rev-parse HEAD`), failing EOS_ERR_CHECKSUM on a mismatch or if HEAD cannot be read. Finding 3 (Medium) -- the exemption's stated reason was "a clone carries its own object hashes". That is true and does not support the exemption: object hashes prove the transfer was not corrupted, not that the content is the content that was reviewed. `git clone --depth 1` takes whatever the default branch points at when it runs, so an unpinned git source is exactly the moving input this function exists to refuse. The pre-network guard now covers git as well, naming "commit" rather than "SHA-256" in the message. Behaviour change worth stating plainly: a git source with no pinned commit is now refused where it used to be cloned. No recipe in this repository uses a git source, so nothing in-tree changes, but an out-of-tree recipe relying on an unpinned clone will need a commit added. Finding 2 (Medium) -- is_git_url() was `strstr(url, ".git") != NULL`, an unanchored substring test. It matched any host under *.github.io, including this organisation's own pages, and any archive whose path contained ".git" -- https://example.com/.github/release.tar.gz took the git branch and skipped the tarball digest check entirely. Now anchored: ".git" at end of string or before /, # or ?, plus the git:// and git@ schemes. Finding 4 (Low) -- "every archive form this fetcher extracts is covered" named three of four; .zip was missing. Added, and the claim narrowed to what it asserts. Verified: ctest 39/39 PASS test_pkg_fetch 7/7 PASS the new tests against the pre-fix fetcher: the git cases FAIL, so they discriminate rather than restate the fix no .git recipe in-tree (grep over *.yaml/*.yml) so the stricter git rule breaks nothing here Refs #115 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>
…rtos (#118) * fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * fix(build): declare the dependency where it belongs, and make the pop-back structural Answers the review on #118. Finding 1 (Medium) -- systems/src/firmware.c includes eos/backend.h and calls eos_backend_find(), but eos_systems linked only eos_core and eos_toolchains; the dependency was satisfied at each consumer instead, so every future user of eos_systems had to rediscover it. eos_systems now declares it, and eos_backends comes off the test_firmware line. Finding 2 (Medium) -- backends/include was added to the global include_directories(), putting a build-orchestration header on the include path of every target in the tree, eos_kernel and eos_hal included. Nothing then stops kernel or HAL code acquiring a dependency master design 5.1 forbids. Taken the reviewer's version rather than a narrower one: the header moves to include/eos/backend.h -- replacing the stale copy that caused the original shadowing -- backends/include is gone, and both the global entry and backends' own target_include_directories with it. include/ stays the single public-header contract, and the shadow is now structurally impossible rather than merely resolved. Finding 3 (Medium) -- the pop-back fired only for seven literal keys, which made it a second authority on what a `system` child key is, kept in step with `case SEC_SYSTEM:` by hand and by nothing else. Add an eighth key there and the swallow returns for it silently. Now structural: every descendant of a system sub-section sits at indent >= 2 and list items `continue` earlier, so indent <= 1 is exactly the condition. Same behaviour on every config under examples/, and it cannot drift. Finding 4 (Low) -- applied the same structural guard to SEC_TOOLCHAIN_LINUX/SEC_TOOLCHAIN_RTOS, which popped back for one sibling and none respectively. A `target:` at toolchain's own indent written after the linux:/rtos: blocks landed in toolchain.rtos_target instead of toolchain.target. Not reachable by any config in examples/ today, which is why it had not bitten. Also dropped the test_crypto_ed25519_loworder registration hunk; this branch is stacked on #127, which is the copy to keep. NOT fixed, and confirmed while testing: finding 5, `system.entry` is parsed by nothing -- no branch in `case SEC_SYSTEM:` and no field on EosSystemConfig -- while six shipped configs set it. examples/multicore-amp/eos.yaml sets `entry: main.c` and `eos info` reports nothing for it. Same failure mode, but it needs a new struct field and a decision about what consumes it, so it is a follow-up rather than something to fold in here. Verified: ctest 40/40 PASS pytest tests/ 15 passed eos info, in examples/industrial-gateway: kind: hybrid / kernel provider: buildroot / RTOS targets (1) which is the parser fix visible end to end -- RTOS targets reads 0 before it. examples/multicore-amp reports 0, correctly: that config has no rtos list at all. Refs #118 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review. --------- Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>
…129) * fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * ci: finish the EoSim fix — a tag that exists, and no wheel anywhere `Simulation Test` and `EoSim Sanity` are both red on master. #106 fixed this class in eosim-sanity.yml but did not reach every site. simulation-test.yml still pins EOSIM_VERSION: "0.1.0". EoSim has no v0.1.0 tag — its releases run v1.0.0 to v3.0.1 — so every run failed at the clone: fatal: Remote branch v0.1.0 not found in upstream origin Bumped to 1.5.0, the release marked Latest and the one eosim-sanity.yml already pins. eosim-sanity.yml still installs a wheel on its Windows and macOS legs: ERROR: HTTP error 404 while getting .../releases/download/v1.5.0/eosim-1.5.0-py3-none-any.whl No EoSim release ships a wheel, so that URL 404s at any version — the version bump alone could never fix these two. The other three jobs in the same workflow already install from a clone; these two were missed. Converted to the same clone + `pip install -e`. Verified, against the real remote rather than from the error message: wheel URL -> HTTP 404 git ls-remote --tags | v0.1.0 -> 0 matches git ls-remote --tags | v1.5.0 -> 1 match clone v1.5.0 + pip install -e -> rc 0 eosim --version -> eosim, version 2.0.0 eosim doctor -> rc 0 (The v1.5.0 tag reporting version 2.0.0 is EoSim's own inconsistency, not a wrong pin — v1.5.0 is what its releases page marks Latest.) Both files still parse as YAML. Stacked on the #127 registration fix so this branch's CI is not red for an unrelated reason. * ci(eosim): pin the commit, assert the version pip resolved, and let the workflow test itself Answers the review on #129. The two diagnoses in the previous commit hold -- there is no v0.1.0 tag and no EoSim release ships a Python artifact -- and this addresses what the replacement pin did not. Finding 1 (High) -- `EOSIM_VERSION: "1.5.0"` is a label, not a version. Re-verified against the remote rather than taken from the review: tag commit committed commit subject pyproject __init__ v1.5.0 7dec3460 2026-05-27 "production-ready v1.4.0" 3.0.1 3.0.1 v3.0.1 b297ec26 2026-05-16 "v3.0.1 — unified ..." 3.0.1 2.0.0 v1.5.0 is the release GitHub marks Latest and the newest tag by commit date, tags a commit whose own message claims v1.4.0, and installs a dist declaring 3.0.1. A reader of `EOSIM_VERSION: "1.5.0"` would believe eos is simulated against EoSim 1.5.0. It is not, and nothing in the file would reveal that. Both workflows now pin `EOSIM_COMMIT` to the full SHA that tag resolves to today, with the table above in the env block and the date it was checked. Not v3.0.1 instead: its `__init__.py` says 2.0.0, so it is only marginally less confusing. A SHA cannot be moved under us; the comment says to go back to a tag once EoSim's tags mean something. Finding 3 (Medium) -- `eosim --version` cannot detect a wrong install. EoSim hardcodes it (`eosim/cli/main.py:49`, `@click.version_option(version="2.0.0")`), confirmed by reading that file at v1.5.0, so it prints 2.0.0 from every tag -- which is exactly why the previous commit could record the version oddity as a curiosity rather than as a failed pin. All three sites now also assert `importlib.metadata.version("eosim")` against `EOSIM_EXPECTED_DIST`, which is the only version the commit pin controls. Finding 4 (Medium) -- this PR could not be validated by CI and was not: both workflows were schedule- and dispatch-only, so neither appeared in this head's checks and all 25 green ones were other workflows. Added `pull_request: paths:` on both files listing both files, so a change to either now runs both. `workflow_dispatch` on the fork is not available to me -- GitHub resolves the workflow on the fork's default branch and returns 404 -- so this trigger is the evidence that is actually obtainable, and it applies to every future change to these files rather than to this one only. Finding 2 (High) -- after this, nothing in the org exercises a published EoSim artifact, and none exists. Verified independently: 13 releases whose assets are promo .mp4 files, a manifest.json and EoSim-guide.pdf; no wheel or sdist at any tag; `https://pypi.org/pypi/eosim/json` returns 404. Converting to a clone is right -- a permanent 404 is not a useful red -- but it would also make the gap green and invisible, which is the "verification whose result is discarded" shape. So `published-artifact-watch` runs on every execution, checks the releases and PyPI, and says out loud that the packaging debt is open. It cannot fail the workflow, deliberately: it is a tracking signal, not a gate on someone else's repo. It emits a `::notice::` the day an artifact appears, which is when it should be deleted and the installs pointed at it. The debt is also stated in the PR body rather than left implied, and raised against EoSim. Finding 5 (Low) -- `/tmp/EoSim` was hardcoded in the two jobs that run on windows-latest and macos-latest, where the Windows job sets no `shell:` and gets pwsh. Now `${{ runner.temp }}/EoSim` everywhere. Finding 6 (Low) -- dropped `-e`. Nothing needs an editable install, it is further from what a developer does in a workflow meant to prove the install works, and it made the dist-metadata assertion depend on the clone staying put. Changed at all five sites, not two, so they are consistent. Also, while in this file: `sanity-gate` had the fail-open shape -- `needs:` five jobs, tested `install-validate` alone, then printed "All EoSim sanity checks passed". Replaced with the `toJSON(needs)` body, which cannot fall out of step with `needs:`, and the new job is included in it. Verified: ctest 39/39 PASS pytest tests/ 15 passed both workflows parse, and structurally: pull_request paths list this file both EOSIM_VERSION gone from every code line both EOSIM_COMMIT is a full 40-char sha both no /tmp/EoSim literal, no `pip install -e` both every `git clone --filter` has a matching `checkout --detach` three importlib.metadata assertions sanity-gate needs == every other job in the file the SHA resolves: refs/tags/v1.5.0 -> tag object 978f215f -> commit 7dec3460cba76083b6645eb4a867e39aa2af97df NOT RUN: neither workflow. `workflow_dispatch` on the fork returns 404 because GitHub resolves it on the fork's default branch. The `pull_request: paths:` trigger should make both run on this PR for the first time -- that will be the first execution either has ever had, and I will report what it shows rather than predicting it. Finding 7 (Low): still stacked on #127; the tests/CMakeLists.txt hunk goes when that lands. Refs #129 * ci(eosim): copy platforms/ in every job that runs a simulation The first real execution of these workflows -- which the pull_request trigger added in the previous commit made possible -- failed, and showed the next blocker behind the missing v0.1.0 tag: FileNotFoundError: [Errno 2] No such file or directory: '/opt/hostedtoolcache/Python/3.12.14/x64/lib/python3.12/site-packages/platforms' on all three Guest OS Install legs and all seven Nested Simulation platforms. platforms/ is data, not package data: eosim/cli/main.py sets PLATFORMS_DIR to <site-packages>/platforms while the wheel ships only eosim/platforms/__init__.py. None of the five jobs that run eosim copied it, so none of them could ever have worked. A tag that exists was necessary and not sufficient, exactly as the review said. Worth recording: this is also what the dropped `pip install -e` was accidentally papering over in the jobs that had a copy elsewhere. With an editable install eosim.__file__ points into the clone, so PLATFORMS_DIR landed beside the checked-out platforms/ by luck. A plain install is the right thing and it made the real dependency visible. install-validate went green in that run and the tracking job reported the packaging debt as intended; the failures were confined to the jobs that actually boot a platform. Verified: both workflows parse; all five eosim-running jobs now copy platforms/ the run that produced the error: 33783625398 (EoSim Sanity), first execution this workflow has ever had Refs #129 * ci(eosim): run the bash steps in bash, including on the Windows legs Second real execution, after the platforms/ fix: ubuntu and macOS now pass Install & Validate; all three windows-latest legs fail. Install & Validate (windows-latest, Python 3.12) # FileNotFoundError: .../site-packages/platforms ##[error]Process completed with exit code 1 The give-away is the leading `#` in that line: pwsh echoed my comment as a command. install-validate is a matrix over ubuntu/windows/macos and set no `shell:`, so on the Windows legs it got the runner default, where `VAR=$(...)`, `cp -r` and a leading `#` are all wrong. windows-sanity had the same gap. `shell: bash` on the six run steps in the two Windows-capable jobs. This is the same defect the review raised as finding 5 for the hardcoded /tmp path -- ${{ runner.temp }} fixed where the clone goes, and this fixes what interprets the script. Worth stating plainly: none of this was visible before the pull_request trigger. Two runs have now found two blockers behind the missing v0.1.0 tag -- platforms/ never copied in any job, and bash steps running under pwsh -- in a workflow that had never executed once. The trigger is doing more work than the pin. Verified: parses; no run step in a windows-capable job is left without a shell run 33784349071: ubuntu + macOS Install & Validate green, windows red with the error above Refs #129 * ci(sim): run the bash steps in bash on the Windows leg too Third run. EoSim Sanity is green -- 23/23 jobs, the first green run that workflow has ever had. Simulation Test has one failure left, `Cross-Platform (windows-latest)`, and it is the same defect I fixed in eosim-sanity.yml one commit ago and did not apply here: Cross-Platform (windows-latest) Install EoSim from source # fails with FileNotFoundError. ##[error]Process completed with exit code 1 pwsh echoing a `#` comment as a command. The cross-platform matrix includes windows-latest and its two run steps set no `shell:`. Refs #129 * ci(eosim): one install recipe, gates that refuse empty input, and a watch that stops posing as a check Answers the overnight review on #129. Finding 3 (Low, and the structural one) -- the five-line install recipe was copy-pasted at seven sites across two workflows, and the PR's own history shows the cost: -e dropped "at all five sites, not two", runner.temp at all of them, shell: bash at several, each a separate round because each site was a separate edit. Now .github/actions/install-eosim owns it -- clone at the pinned commit, install, stage platforms/, assert the dist version -- and all seven sites are a four-line `uses:`. The platforms/ workaround and the version assertion get one place to be deleted from when EoSim fixes its packaging. The triplicated inline importlib.metadata assert is gone with it. Finding 5 (Low), folded into the composite -- SITE_PACKAGES comes from sysconfig.get_paths()["purelib"], which needs no import and cannot be shadowed, matching eBoot's copy of the workaround instead of solving the same problem a second way. Finding 1 (Medium) -- the inline gate accepted nothing as everything: an empty RESULTS printed "All EoSim sanity jobs succeeded." and exited 0, and null produced a raw jq error, where eos#121's extracted ci-gate-check.sh refuses both. Both gates now refuse empty/null input with a diagnosis before jq runs, under set -euo pipefail, and carry a note to become a call to #121's script once it is on master so the two cannot drift. Reproduced the review's side-by-side on the hardened body: "" rc=1 gate received no results to check (was rc=0 PASS) null rc=1 gate received no results to check (was raw jq error) {} rc=0 PASS -- shared with #121's script; reported there, and [] rc=0 PASS unreachable here: needs has five entries skipped rc=1 FAIL: a: skipped success rc=0 PASS Also applied to simulation-test.yml's Simulation Gate, which still had the original fail-open shape -- needs: [simulate, cross-platform], a test of `simulate` alone, then "All simulation tests passed". Same toJSON(needs) body, same guards. Finding 4 (Low) -- published-artifact-watch could not fail and sat in sanity-gate's needs under a name that made a green tick assert a deficiency. Now "EoSim artifact publication (informational)", schedule/dispatch only, out of the gate: pull requests no longer carry a permanently-green status about another repository's release process, and the debt still gets said out loud every night. Finding 6 (Low) -- simulation-test's EOSIM_COMMIT comment argued tag selection for what is now a SHA; it now states the actual reason (EoSim's tag names do not identify their contents) and gains the EOSIM_EXPECTED_DIST the composite asserts. Finding 2 (Medium) is a two-line change in eos#121's registry, not here -- both workflows need NOT_REQUIRED entries with a paths-filter reason once both PRs land, or #121's classification test fails. Posted on both threads with the exact entries; not folded in, because that registry lives on #121's branch and this PR must not carry another PR's hunks. Verified locally before push: all three YAML files parse composite referenced at 7 sites; zero inline clone/install/copy remnants eosim-sanity gate needs == every job except itself and the informational watch; sim gate iterates toJSON(needs), no single-dependency branch the five-input table above, executed the workflows themselves: this push's pull_request run is the test, and I will report what it shows rather than predict it. Refs #129 * ci: pin the eBoot and ebuild checkouts instead of floating on master Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot #94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot #94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from #135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to #135's commit -- git drops it as already-upstream the moment #135 merges. The pin itself is #135's to review.
The pin in the previous commit stops an eBoot or ebuild master that does not compile from turning every eos pull request red. It also stops that break being visible from here at all, which on its own is the worse half of the trade: the next upstream breakage would be silent instead of loud, and the pin would sit at a172a6d indefinitely because nothing would ever ask -- while quietly dropping the six eBoot commits that follow it. `.github/workflows/upstream-drift.yml` is the other half. It builds eBoot at master and compares both pins against their master, so an upstream break still surfaces, on a run that blocks no pull request and names the repository responsible. Design choices, each one avoiding a defect this repo has been filing against: Its own file, schedule + workflow_dispatch only. The first draft was a job inside eos-simulation.yml guarded by `if: github.event_name == 'schedule'`. That works, but the run showed it on the PR as a SKIPPED check -- and a required-check gate that treats any non-success as a failure, which is the design in embeddedos-org#121, would trip on exactly that. Moving it out removes the check from PR runs entirely rather than relying on every future gate to exempt it. Not `continue-on-error`. A job reporting success over a failure is the fail-open shape we keep finding in aggregating gates. This one may fail honestly precisely because it gates nothing. Failing is reserved for "upstream does not build". A pin merely behind a healthy master is a warning: actionable, not broken. The pins are read out of eos-simulation.yml, not copied. A second copy of a SHA is a second thing to forget. The read demands a 40-character hex value and fails closed if it cannot find one, so a renamed or reformatted key stops the job instead of silently letting it check a pin of "". The failure message is scoped to the build step's own outcome, so a checkout, apt or pin-read failure cannot report itself as evidence about eBoot's master. The summary says eBoot is built and ebuild is only compared, rather than implying both were checked. Verified by execution, not by reading: eBoot @ a172a6d (the pin) host build rc=0, 0 errors eBoot @ 22d8f8b (master) host build rc=2, "no member named 'reserved' in 'eos_image_header_t'" at eos_image.h:135 and :142 -- the same defect CI reports, so the pin is demonstrably the difference pin extraction, run against the real file and two mutations of it: real workflow -> both SHAs read correctly, and the eBoot one resolves to embeddedos-org#86's merge key renamed to EBOOT_SHA -> fails closed, rc=1 value unquoted and shortened -> fails closed, rc=1 drift logic, run against the real repositories: pin 6 behind healthy master -> "behind by 6" warning, no failure pin absent from master -> "not an ancestor" warning, exit 0 under set -e (the first draft printed "pinned pin not found on master commit(s)") pin equal to master -> no warning eos on this branch: cmake build rc=0, ctest 39/39, pytest 15/15 with the new workflow file present, test_crypto_ed25519_loworder registered and passing. When eBoot embeddedos-org#94 lands, bump EBOOT_COMMIT to that merge; this job is what will say whether it holds.
Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.
Description
Problem
kernel/src/sync.candkernel/src/task.cdo not compile onmaster.#67 ("Correct and transitive mutex priority inheritance") replaced the
save/restore priority-inheritance scheme with one that recomputes an owner's
effective priority from an invariant. #55 ("restore mutex owner priority after
waiter timeout") was written against the pre-#67 tree and landed on top of it,
putting half of the old scheme back without removing the new call sites.
The file that came out references neither design completely —
sync.ccallstask_valid(),pi_propagate()andg_blocked_on[], which #55 deleted, whilemtx_recompute_owner_priority()readsmtx_t::original_prio, a field #67removed from the struct:
kernel/src/sync.c:66:21: error: 'mtx_t' has no member named 'original_prio'
kernel/src/sync.c:149:9: error: implicit declaration of function 'task_valid'
kernel/src/sync.c:150:9: error: 'g_blocked_on' undeclared
kernel/src/sync.c:197:13: error: 'g_blocked_on' undeclared
kernel/src/sync.c:206:5: error: implicit declaration of function 'pi_propagate'
kernel/src/sync.c:225:13: error: 'g_blocked_on' undeclared
kernel/src/task.c:576:6: error: redefinition of 'eos_task_set_current_internal'
The
task.credefinition is a second, independent collision: #55 added aeos_task_set_current_internal()that #67 had already added higher up the file,and a matching second declaration in
kernel_internal.h.This is why the Host Tests (x86_64) job in
build.ymlhas been failing onmasterat its very first step — that job compiles exactly these files with-Werror.Changes
kernel/src/sync.c— restored #67's design, which the kernel documentationalready describes (
docs/book/part2-kernel-hal/ch06-kernel.md:206,docs/api-reference.md:117) and whichtests/test_priority_inherit.calreadytests:
effective = min( base_priority,
priority of every task waiting on every mutex the task holds )
g_blocked_on[],task_valid(),pi_recompute()andpi_propagate(); droppedmtx_recompute_owner_priority()and theboost-on-lock block that depended on the removed
original_priofield.mtx_remove_waiter()returns whether it removed anything again.kernel/src/task.c— removed the duplicateeos_task_set_current_internal(). Kept #67's version, which clearsg_currenton an out-of-range handle and marks the task
RUNNING, matching what the headerdocuments; the duplicate silently returned instead.
kernel/include/eos/kernel_internal.h— removed the duplicate declarationand folded both doc comments into the one that remains.
CHANGELOG.md— added the build entry, and corrected the existingeos_mutex_lockentry, which described restoringoriginal_prio— a field thatno longer exists.
Why this design and not #55's
#55's acceptance criteria are both still met, by construction rather than by a
second mechanism:
removed from the queue and
pi_propagate()recomputes from the waiters thatremain. Because the boost is derived from the live waiter set instead of
being stashed in the mutex, there is nothing left to leak.
EOS_KERN_NO_MEMORYis returned before the waiter is enqueued.Restoring #67 additionally puts the code back in agreement with the shipped
documentation, which still describes the transitive design, and with
test_priority_inherit.c, which was added by #67 and tests it directly.Testing
No new tests — this restores an implementation whose test suite is already in
the tree and was passing when it was written. Both suites run and pass now:
tests/test_priority_inherit(6 tests)tests/test_kernel(10 tests, incl.test_mutex_pi_timeout_restores_priorityadded by #55)build.ymle2e step verbatim:gcc -std=c11 -Wall -Wextra -Werror -O0 -g -Ikernel/include -Iinclude -o build_e2e tests/test_e2e.c kernel/src/task.c kernel/src/sync.c kernel/src/ipc.c kernel/src/mem/heap.cALL 88 CHECKS PASSEDmasterNote that
test_mutex_pi_timeout_restores_priority, the test #55 added for itsown fix, passes against the restored #67 implementation — direct evidence that
the behaviour #55 wanted is preserved.
Verified with GCC 6.3.0 (MinGW), CMake 4.4.3, Ninja 1.13.0 on Windows.
Notes for the reviewer
Stacked on PR 1 (
fix/cmake-duplicate-test-targets), which is needed beforeCMake will configure the test suite at all. One build breakage remains after
this PR —
core/src/log.c, handled in the next PR — so CI here is still red atthat file.