fix: guard three test dereferences that follow a non-stopping null assertion

[srpatcha]

Problem

Three test files dereference a pointer on the line immediately after the
assertion that checked it for NULL:

• tests/test_registry.c:31-32 ΓÇö EAPPS_ASSERT(e != NULL, ...) then strcmp(e->info.name, ...)
• tests/test_game_engine.c:54-55 ΓÇö EAPPS_ASSERT(g != NULL, ...) then g->width, and the rest of test_game_lifecycle() keeps dereferencing g
• tests/test_eremote.c:63-64 ΓÇö ASSERT_NOT_NULL(s, ...) then s->name

Root cause

The assertion macros do not stop. tests/test_registry.c:6-9 and the shared
tests/eapps_test.h:77-82 both expand to

do { if (!(cond)) { fprintf(stderr, ...); failures++; } else { passes++; } } while(0)

ΓÇö no return, no abort. A failed null check therefore records the failure
and falls straight through to the dereference.

The consequence is that the suite is least informative exactly when it has
something to say: when one of these lookups actually returns NULL, the
process segfaults on the next line, the FAIL: message is lost with the
unflushed stream, every later test in the file never runs, and ctest reports
a crash instead of the assertion that explains it.

Surfaced by the maintenance health scan, §"C — unused functions and dead code"
for eApps ΓÇö the three nullPointerRedundantCheck warnings
(Either the condition '(e)==NULL' is redundant or there is possible null pointer dereference). Recorded in the sweep backlog on 2026-09-06 as a P2.

Fix

Guard the dereference at each of the three sites, without touching the macros:

• test_registry.c ΓÇö fold the null check into the follow-up assertion
  (e != NULL && strcmp(...) == 0). The test lives in main(), so returning
  early would skip the summary line and the exit status derived from it.
• test_game_engine.c and test_eremote.c ΓÇö if (ptr == NULL) return; after
  the assertion, which is what the rest of each function needs.

The assertion that reports the failure is left in place in all three files, so
a regression still produces a FAIL: line and a non-zero exit ΓÇö it just
produces it instead of a segfault.

Files changed

• tests/test_registry.c
• tests/test_game_engine.c
• tests/test_eremote.c

Expected impact

No change to any passing run: all three guards are unreachable while the
lookups succeed, which is why the recorded ctest result is identical to the
baseline (11/11). The change is only visible on a failing run, where the suite
now reports the failure rather than crashing before it can.

Risks and compatibility

Test-only; no production source is touched and no public interface changes.

The narrower, deliberate scope: this does not repair the underlying
non-stopping assertion macros, and it does not de-duplicate the seven local
copies of EAPPS_ASSERT that tests/eapps_test.h already provides. Both are
recorded separately in the sweep backlog (2026-09-06, P2 and P3). Per
CLAUDE.md ΓÇö "Restructure and change behaviour in separate commits" ΓÇö that
consolidation belongs in its own change, and it would want a maintainer to
decide whether the macros should return, abort, or stay as they are.

Note also that .github/workflows/ci.yml:42 runs ctest without
--no-tests=error, which CLAUDE.md requires; that is a separate finding and
is not addressed here.

Verification

Executed in an isolated worktree branched from origin/master:

Check	Result	Duration	Command
build	pass	0s	cmake --build build/tests --parallel 4
cppcheck	pass	1s	/home/srpatcha/eos/.ai/toolchains/run-in-toolchain.sh /home/srpatcha/eos/.ai/autoreview/state/worktrees/eApps__test-null-deref-after-failed-assert -- cppcheck --enable=warning,portability --inline-suppr --error-exitcode=2 tests/test_registry.c tests/test_game_engine.c tests/test_eremote.c
ctest	pass	0s	ctest --test-dir build/tests --output-on-failure --no-tests=error --parallel 4

---

Opened by the scheduled autoreview pipeline (model claude-opus-5), branched from origin/master. No human has reviewed this yet. Close it freely if the fix is wrong - a bad automated PR is a bug worth reporting.

Fixes #46

[srpatcha]

Review — eApps#45 "fix: guard three test dereferences that follow a non-stopping null assertion"

head: 427dffc author: srpatcha ci: fail — 3 red of 13; 2 of the 3 are red on master too

Conflict of interest, declared up front. This PR was opened by this pipeline —
branch autofix/test-null-deref-after-failed-assert, and the body's own footer says so.
.ai/reviewer.md is explicit: "If you implemented it, you do not approve it. This is the
one role boundary worth enforcing structurally." So this is not an approval and must not
be read as one; post-review.sh posts a plain comment and cannot approve in any case. What
follows is an independent re-verification from a clean export — every number below was
measured again rather than copied from the PR body — plus the findings, which are about the
PR's CI state and the process that produced it. A human still has to review the three-line
change itself. It is a draft, so nothing merges on this report.

What I verified independently: the branch is origin/master plus one commit, three files,
+11/−1. cppcheck on origin/master's tests/ reports the three nullPointerRedundantCheck
warnings at exactly the lines the body names — test_eremote.c:64, test_game_engine.c:55,
test_registry.c:32. cppcheck on this head reports zero of them, with the same unrelated
pre-existing integerOverflow on both sides. ctest is 11/11 here, and the reasoning about the
macros is correct: test_registry.c:6-9 and tests/eapps_test.h:77-82 both record the failure
and fall through with no return and no abort.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	PR body; .ai/autoreview/fix-submit.sh	policy / Policy / Linked Issue is red, and it is the only red check this PR is responsible for. The body has no Closes #NN / Fixes #NN line — it cites "the maintenance health scan, §C" and "the sweep backlog on 2026-09-06", neither of which is a GitHub issue. This is not a one-off: the body is generated by fix-submit.sh from the template the brief specifies (the finding, the review that surfaced it, what was run, what was not), and that template has no slot for a linked issue, so every autofix PR this pipeline opens fails the same check in the same way — the same policy this run reviewed two PRs about (ebuild#144, and its upstream embeddedos-org/.github#9). An automated PR that is structurally incapable of passing a required policy check is a defect in the generator, not in the change.	Two options, and the choice is a maintainer's. Either have the fix pipeline open (or reuse) a tracking issue per finding and emit Closes #NN in the body — the sweep backlog already has the content, it just is not in GitHub — or add autofix/* to the policy's exemptions upstream in linked-issue-policy.yml, alongside the state guard .github#9 is adding. The first is better: it gives the backlog item a public home and keeps the policy uniform. Until one of them lands, this PR and every one like it sits BLOCKED on a check it cannot clear.
2	Low (P3)	.github/workflows/ci.yml:42 and :92	ctest --test-dir build --output-on-failure -C Release and the Sanitizers lane's ctest --test-dir build --output-on-failure both run without --no-tests=error, so a build that registers no tests reports success. .ai/security.md names this exact case ("ctest exits 0 when it finds no tests at all… always --no-tests=error"), and the org proposal §28.2 draft in proposals/2026-09.md (2026-09-02) requires it. The PR body raises this itself and explicitly declines to fix it, which is the right scope call — recorded here so it is tracked rather than mentioned and lost. Measured at this head: ctest returns 11/11 with and without the flag, so adding it changes nothing today; it is a guard against a future in which the tests stop being built.	One flag on both lines. Not this PR's job; worth its own one-line change, and it is the kind of thing the brief's autofix rule was written for.
3	Low (P3)	tests/eapps_test.h:77-82; seven local EAPPS_ASSERT copies	The fix is three guards on a class defect, and the class is untouched. The macros still record-and-continue, and grep -rln "define EAPPS_ASSERT|define ASSERT_NOT_NULL" tests/ returns eight files — eapps_test.h plus seven local redefinitions in test_string_utils.c, test_registry.c, test_prefs.c, test_date_utils.c, test_game_engine.c, test_math_utils.c, test_expr_parser.c. The next test file that asserts non-null and dereferences on the following line reintroduces the defect, and cppcheck will find it one sweep later. The body says this deliberately and defers it, citing CLAUDE.md's "restructure and change behaviour in separate commits" — correct, and the deferral is the finding, not the decision.	A maintainer decides whether the shared macro should return, abort, or stay. ASSERT_NOT_NULL in particular has a defensible return semantic that EAPPS_ASSERT does not, since its whole purpose is to gate a dereference. Then delete the seven copies. Recorded in the sweep backlog (2026-09-06, P2 and P3) per the body.

Not findings:

• Native (windows-latest) and JS/TS lane are red, and neither is this PR's. I checked
  master's own latest CI run (34664911413): both jobs are failure there too. Windows fails
  on three MSVC errors in apps/ewifi/ewifi_passwords.c — C2440: cannot convert from 'ewifi_security_t' to 'const char *' at (115,35) and (118,29), C2109: subscript requires array or pointer type at (119,31) — in a file this PR does not touch. JS/TS fails at
  npm ci with EUSAGE: "can only install with an existing package-lock.json", plus an
  ERROR Expected object but found - string before it. master is red in eApps, which is a
  finding against the repo and not against this change; the 2026-09-03 proposal "Nothing in the
  design requires the trunk itself to build" already covers the shape of it.
• The three guards are unreachable on a passing run, which is why ctest is identical to the
  baseline. Verified: 11/11 at both origin/master and this head.
• test_registry.c's fix differs in kind from the other two — folding e != NULL into the
  next assertion rather than returning — and the comment says why (the code is in main(), so
  an early return would skip the summary and the exit status derived from it). The cost is that
  a NULL lookup now reports two failures, the second labelled find test1 name, which is
  a slightly misleading message for a lookup that returned nothing. Acceptable, and better than
  the segfault it replaces.
• test_eremote.c's ASSERT_STR_EQ(s->name, …) is the last statement in test_scenes(), so
  the return skips nothing. test_game_engine.c's return skips the remainder of
  test_game_lifecycle() — all of which dereferences g — and later test functions called
  from main() still run, which they did not when the process crashed.
• Nothing was weakened. No test disabled, no assertion removed, no lint loosened. Every
  EAPPS_ASSERT that reported a failure before still reports it.
• Incidental, present on master and untouched here: cppcheck reports
  tests/test_performance_benchmarks.c:16:20: error: Signed integer overflow for expression 'i*i' [integerOverflow] — dummy += i * i with int i up to 10,000,000. That is undefined
  behaviour in a benchmark harness, it is a cppcheck error rather than a warning, and it is
  the only other thing cppcheck finds in tests/ on either side. Not this PR's; recorded so it
  is not lost.

Architecture conformance

Conforms, and §5.1 is barely engaged. §21 places eApps in Tier 4 — Developer Ecosystem
(Registry/eApps); all three files are under tests/ in the owning repo, so §21.1 is not engaged
either. The only include touched is the existing "../core/common/include/eapps/registry.h" in
test_registry.c, which is within-repo and downward — no new import, link line or manifest
entry, and nothing points up a tier. No production source is compiled differently: cmake --build output is identical in content, and §23.2's compatibility contracts are untouched
because no public interface changes.

The clause this serves is §28's evidence model, from the same angle as eos#160 in this run: a
suite that segfaults before printing its FAIL: line produces a crash where it should produce a
diagnosis, and ctest then reports the crash instead of the assertion that explains it.

No new proposal appended. Finding 2 is covered by the 2026-09-02 §28.2 draft; finding 1 is
covered by the 2026-09-14 "Shared CI/governance policy: the caller/reusable-workflow contract
is undefined" proposal, which is the right home for an autofix/* exemption; the red trunk is
covered by the 2026-09-03 "Nothing in the design requires the trunk itself to build". Nothing
at this head reveals a further gap in the master design.

Proposed changes

1. Resolve finding 1 before this or any autofix PR can go green: either emit
   Closes #NN from fix-submit.sh (and file the backlog item as an issue), or
   exempt autofix/* upstream in linked-issue-policy.yml. This is the blocker.
2. Take the draft off draft only once 1 is settled; the other two red checks
   are master's and will not clear on this branch.
3. Separately, not here: --no-tests=error on ci.yml:42 and :92 (finding 2).
4. Separately, not here: decide the macro semantics and delete the seven
   duplicate copies (finding 3).

No fix PR opened, for two reasons. Findings 2 and 3 are Low and the brief's autofix rule is
High-only. Finding 1 is Medium and, more to the point, opening an automated PR to fix the
automated-PR generator — from inside the same run that is reviewing its output — is exactly the
recursion .ai/reviewer.md's role boundary exists to prevent. It needs a human.

Verification performed for this review

The eApps checkout has a clean working tree (git status --short empty, before and after)
and was not modified — no worktree, checkout, stash or reset. The PR head was fetched
read-only into refs/autoreview/pr45. The build and ctest ran against git archive 427dffc3 | tar -x into a temporary directory outside the repository; cppcheck ran through
.ai/toolchains/run-in-toolchain.sh, which requires its workdir under /home/srpatcha/eos, so
two exports were made under .ai/autoreview/state/scratch/ and removed afterwards. Nothing was
committed or pushed, and nothing was pushed to this PR's branch.

Check	Result
git log --oneline origin/master..427dffc3	one commit. git diff --stat origin/master 427dffc3 → 3 files, +11/−1. The body's scope claim is exact
cmake -B build -DBUILD_TESTING=ON → cmake --build build --parallel 4	PASS
ctest --test-dir build --output-on-failure (CI's invocation, ci.yml:42)	PASS — 100%, 11/11, 0 failed. Matches the body's baseline
ctest --test-dir build --output-on-failure --no-tests=error	PASS — 11/11. Identical; the missing flag changes nothing today (finding 2)
cppcheck on origin/master's tests/, --enable=warning,portability --inline-suppr	3 nullPointerRedundantCheck — test_eremote.c:64 ((s)==NULL), test_game_engine.c:55 (g!=NULL), test_registry.c:32 (e!=NULL) — exactly the three the body names, plus one unrelated integerOverflow
cppcheck on this head's tests/, same flags	0 nullPointerRedundantCheck. Only the same integerOverflow remains. The fix does what it claims
tests/test_registry.c:6-9 and tests/eapps_test.h:77-82	Both expand to if (!(cond)) { fprintf(...); failures++; } else { passes++; } — no return, no abort. The root-cause claim is accurate
grep -rln "define EAPPS_ASSERT|define ASSERT_NOT_NULL" tests/	8 files — the shared header plus seven local copies (finding 3)
.github/workflows/ci.yml:42, :92	ctest without --no-tests=error on both (finding 2)
master's own CI run 34664911413	Native (windows-latest) failure, JS/TS lane failure, all nine other jobs success. Two of this PR's three red checks are inherited
Native (windows-latest) failure cause on this head	apps/ewifi/ewifi_passwords.c(115,35) error C2440, (118,29) error C2440, (119,31) error C2109 — a file this PR does not touch
JS/TS lane failure cause on this head	npm error code EUSAGE — npm ci with no package-lock.json; preceded by ERROR Expected object but found - string
policy / Policy / Linked Issue	failure. The body contains no Closes/Fixes reference (finding 1)
PR metadata	draft: true, mergeable: MERGEABLE, mergeStateStatus: BLOCKED, reviewDecision: REVIEW_REQUIRED

Not checked

• Nobody independent has reviewed the change itself. This is the stated limitation and the
  reason for the notice at the top. Everything above re-measures the PR's claims; none of it
  substitutes for a maintainer reading three guards written by the same pipeline that is now
  reporting they are correct.
• Windows and macOS — NOT BUILT. Only the Linux host build and ctest ran. macos-latest
  and ubuntu-latest are green in CI; windows-latest is red for the pre-existing ewifi
  reason, so whether these three files compile clean under MSVC is covered by CI only and
  the leg that would say so is failing before it gets there.
• The Sanitizers lane was not reproduced locally. It is green in CI on this head. ASan
  would be the thing that catches a missed guard, and I did not run it.
• The failure path was not provoked. I did not force eapps_registry_find,
  eapps_game_create or eremote_scene_get to return NULL to watch the guards fire, so the
  claim that the suite now reports instead of crashing is Inferred from the source, not
  observed. That is the one behaviour this PR exists to change, and it is the obvious thing for
  a human reviewer to demand a demonstration of. It would take a one-line stub, the way eos#160
  in this run proves its own macro.
• The three red checks were not re-run. I read the recorded conclusions and the failing job
  logs; I did not re-trigger anything.
• The eight assertion-macro copies were not diffed against each other. I counted the files;
  whether the seven local definitions are identical to eapps_test.h's or have drifted is
  unexamined, and it matters for finding 3's cleanup.

---

Automated architecture review of 427dffc3472d — 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]

Resolved the contribution-caused linked-issue policy failure without changing the branch.

• Head remains 427dffc3472dd22505a1029e8ecd52acb11be04f.
• Created and linked Tests dereference NULL after non-stopping assertions #46 with Fixes #46.
• Re-ran https://github.com/embeddedos-org/eApps/actions/runs/34894392698: Linked issue policy completed successfully.