Test suite rebuild

[NomDeTom]

Native test suite: per-suite isolation, leftover-state reporting, and the coverage that would have caught it

Longitudinal PR off develop, phased, with a commit per stage. Test and harness work plus two small production changes, each isolated and explained below.

Why

test_admin_radio failed four assertions that had nothing to do with the code they were testing. The cause was 22 suites upstream.

Every native suite that constructs a NodeDB loads and saves ~/.portduino/default/prefs/ — nodes.proto, config.proto, channels.proto, module.proto, device.proto, warm.dat, transmit_history.dat — and nothing ever cleared it. test_nodedb_blocked deliberately fills the database with MAX_NUM_NODES - 2 favourited nodes to test the protected-node cap; that is correct, and it is what the test is for. An incidental removeNodeByNum() in the next test persisted it. Twenty-two suites later test_admin_radio loaded a full, all-protected database in which getOrCreateMeshNode() can only return NULL, so the node under test was never created and the protected-bit assertions read false.

Three things made this expensive:

• The poison outlived the run. A filtered rerun of test_admin_radio alone kept failing, in a different worktree, with a fresh .pio — the state is keyed on $HOME.
• Per-run isolation would not have fixed it. A full run from a completely empty $HOME reproduces it, because the poisoning is generated within one run.
• Nothing looked at what a suite left behind. There was no signal at all until a distant suite failed for reasons that looked unrelated.

CI never saw it, and not because CI is careful: its area_rules happen to run admin first, while PlatformIO's local discovery is reverse-alphabetical and runs it last. Neither order was chosen. CI was green by accident, and would have broken the moment anyone reordered the areas.

A test run could also rewrite a real meshtasticd node database on the same machine.

What landed

Phase 0 — one canonical node cap

The native cap was stated in four places that disagreed, and that disagreement already caused a wrong diagnosis: the cap was computed as 248 from mesh-pb-constants.h, so a genuinely saturated 200-node database looked arithmetically impossible. The real cap is 198.

On portduino MAX_NUM_NODES is not a compile-time constant at all — the variant defines it as portduino_config.MaxNodes, resolved at runtime, default 200, settable per host via General: MaxNodes. variant.h is reached before mesh-pb-constants.h, so that header's ARCH_PORTDUINO branch never fires and its plausible-looking 250 is dead code.

#error-guarding that dead branch found a real defect. Eight translation units reach mesh-pb-constants.h without configuration.h — SerialConsole.cpp, StreamAPI.cpp, PacketAPI.cpp, ServerAPI.cpp, PiWebServer.cpp, ServiceEnvelope.cpp, MeshtasticOTA.cpp and test/TestUtil.cpp — so each was compiling with a different MAX_NUM_NODES, and therefore a different PACKETHISTORY_MAX, than the rest of the build. Each now includes configuration.h first. It cannot be included from mesh-pb-constants.h itself: that reaches SerialConsole.h via DebugConfiguration.h and closes a cycle.

Also names the bare 250 in getMaxNodesAllocatedSize() as NODEDB_MIGRATION_LOAD_CEILING — a decode allowance for files from larger-cap firmware, not a cap — and fixes the doc that named the wrong source and a "10-250" range wrong for native.

Phase 1 — per-suite scratch $HOME, and leftovers as an outcome

bin/pio-test-isolate.sh runs each suite in its own scratch $HOME, registered as test_testing_command for env:native and env:coverage so a bare pio test and CI get the same boundary, not just bin/run-tests.sh. Mutation inside a suite is free; carrying state across a suite boundary is now impossible by construction rather than by policy.

Overriding HOME around the already-built binary rather than around pio also sidesteps the blocker that a bare HOME= breaks pio's own ~/.platformio/penv/bin/pio lookup.

Leftovers are reported on a second axis, PASS/FAIL × CLEAN/DIRTY, because an unintended write has no matching assertion by definition — nobody writes TEST_ASSERT for a save they do not know is happening. Five decisions, each because the obvious implementation is brittle enough to get deleted:

1. The harness asserts it, not the test, so it applies to every suite without the author opting in.
2. Only the set of changed paths is asserted, never contents. Hashes answer "did this change?" and nothing more. Content baselines over protobuf bytes are snapshot tests — add a field to NodeInfoLite and every recorded hash churns, which is how snapshot suites become an --update-all ritual and then noise.
3. Granularity follows the state flag, so the two ship together. Per-test by default (TestUtil redefines RUN_TEST to checkpoint after each test, naming the exact test that dirtied things); suite boundary for state=per-suite, where carrying state across test cases is the declared behaviour.
4. A declared write that does not happen is MISSING, reported separately from DIRTY. It catches silently-broken persistence — the shape of the upstream TAK config bug, where a has_ flag was never set so the save wrote nothing and no test noticed. A warning for now, since some declared writes are conditional.
5. Guard the guard. state_assert_empty() refuses to run a suite against a sandbox that is not empty — otherwise the after-diff measures against the wrong baseline and reports CLEAN while meaning nothing — and bin/test-state-check.sh drives the real wrapper with fixtures asserting CLEAN / CLEAN / DIRTY / MISSING plus both directions of the empty assertion.

Declarations live in one central test/state-manifest.tsv — suite, flags, mandatory reason — so every opt-out is visible in one diffable list; per-suite files hide growth. The run prints how many suites declare non-default handling, so the number creeping up is visible without anyone auditing the file. --write-manifest proposes entries for a human to paste and justify; it never applies them and neither does CI, because an auto-accepted baseline is the same rot as an auto-updated snapshot.

Graded AMBER, not RED: with isolation in place, DIRTY means "undeclared", not "dangerous", and a check that lands red on day one gets switched off.

Also: a signal name from the runner is not a crash. exit(UNITY_END()) returns the failure count, and PlatformIO renders a non-zero exit code as a POSIX signal — 4 failures prints SIGILL, 5 prints SIGTRAP, and the suite is reported [ERRORED]. That cost hours of hunting a memory bug that did not exist, on an env that carries no sanitizer at all. The runner now says so inline.

Phase 2 — test_admin_radio fixtures per test

setUp() did if (!nodeDB) nodeDB = new NodeDB(); and never deleted it, so 83 of 85 tests shared one never-reset database and never restored config, owner, devicestate or channelFile. The fixture that does restore them was opt-in and armed by exactly two tests. The comment claiming the rest "set their own config/region state and are unaffected" was not true — the admin handlers under test write all four.

Every test now goes through the fixture. All 85 passed, so nothing was silently relying on the shared state. It costs about 7% of the suite's runtime, worth paying to write the phase-3 tests against a clean fixture rather than 83 tests' residue.

Phase 3 — the coverage

• test_fscommon_getfiles, 8 tests. getFiles() runs on every phone sync and nothing asserted its cap, depth limit, wasLimited paths, overlong-path rejection or capacity release. All eight describe today's behaviour and pass on arrival — fix(phone-api): forward-port manifest-scan skip + bounded getFiles (#10754, #10757) to develop #10778 already landed the real fix — which is the point: this is the baseline a later change has to leave alone.
• Three admin node-DB tests. set_favorite_node, set_ignored_node and toggle_muted_node persist a NodeInfoLite bit and must not reconfigure the radio. Pure characterization — they pass on develop — but that reconfigure is the path implicated in the WisMesh Tag favourite crash, and develop asserted nothing about it.
• menuHandler::toggleNodeMuted() extracted from its banner-callback lambda, which is why nothing in MenuHandler.cpp was reachable from a test. Behaviour-neutral by construction.

Phase 4 — randomised order, last and deliberately

Randomising an order-dependent suite set does not find bugs so much as convert a silent pass into intermittent red, and the first instinct is to revert the randomisation rather than fix the coupling. Phases 1–2 removed the coupling; this keeps it removed.

--shuffle / --seed <n>, seeded from HEAD by default: one order per commit, so a red is replayable and attributable to the diff instead of flaky. The seed is printed at the start and carried into the RESULT: line, so a verdict is replayable from that line alone; the full order prints on failure, because for an order-dependent failure the order is the diagnostic. The shuffle is a Fisher-Yates over a MINSTD generator rather than awk's rand(), whose sequence differs between gawk and mawk — a seed that does not reproduce the same order on another machine is not a seed.

CI shuffles its area order the same way, seeded from GITHUB_SHA and printed with the command to replay it locally.

Lessons learnt along the way

• PlatformIO ignores the order of -f flags. list_test_names() walks test_dir with os.walk(); filters only select. So per-suite isolation did not need one invocation per suite — a test_testing_command wrapper gets it in a single invocation, which is the faster mechanism (measured: 54s vs 68s for three suites, ~4.7s of pio startup per extra invocation). But randomised order does need per-suite invocations. That ~4.7s/suite is the real price of phase 4, and it is worth stating rather than discovering later.
• ARDUINO is defined on portduino, so it is the wrong guard for host-only test code. The state-checkpoint code silently compiled to a no-op stub until it was guarded on ARCH_PORTDUINO.

Results

Run	Verdict
-e native, pristine HOME (run 1)	AMBER 16 suite(s) left undeclared shared state — all 44 suites PASS
-e native, normal HOME (run 2, after populating the manifest)	GREEN 44/44 suites passed, all CLEAN
-e native --shuffle (seed 809606438)	GREEN 44/44, all CLEAN
-e native --seed 20260801	GREEN 44/44, all CLEAN
-e coverage (informational)	RED 2 failed — pre-existing, not this branch

The 16 undeclared suites on run 1 match the measured baseline exactly. Each now carries a manifest entry with a reason, taken from --write-manifest output rather than guessed.

The coverage failures are pre-existing and unrelated: test_packet_signing's B11/B12, which fail on this host under ASan on develop too. They are a latent uninitialised-read/UB in the PKI encode path that this host's ASan exposes and CI's older runtime does not; ruled out as prefs-related by running from a pristine HOME. 43 of 44 suites pass under coverage.

Not in this PR

Excluded	Why
reserve() outside the __cpp_exceptions guard; file.name() null guard	Production changes; stacked separately so the FS suite here can show they alter nothing observable. Neither is coverable natively — exceptions are on, so the #else branch is not compiled, and no in-tree backend returns a null name.
Narrowing the mute toggle's saveToDisk() mask	The defect phase 3c characterizes. Today one NodeInfoLite bit rewrites all five segments, and the test asserts that deliberately, with a comment naming it. When the narrowing lands the assertion is expected to change — that diff is the point.

🤝 Attestations

• I have tested that my proposed changes behave as described.
• I have tested that my proposed changes do not cause any obvious regressions on the following devices:
  • Heltec (Lora32) V3
  • LilyGo T-Deck
  • LilyGo T-Beam
  • RAK WisBlock 4631
  • Seeed Studio T-1000E tracker card
  • Other (please specify below)

Bench test suite only.

Summary by CodeRabbit

• New Features
  • Native deployments support runtime-configured node capacities with validation for unsupported values.
  • Node muting now persists reliably and safely ignores unknown nodes.
• Bug Fixes
  • Standardized node and packet ID formatting in menus and diagnostic logs.
  • Improved test isolation and detection of undeclared filesystem changes.
• Tests
  • Added coverage for file enumeration, node controls, defaults, and shared-state handling.
  • Added reproducible suite shuffling and clearer failure diagnostics.
• Documentation
  • Expanded native testing guidance and clarified node-capacity behavior.

---

Also finishes the node-ID log-format cleanup started in #10798. That PR documented the convention (0x%08x in logs, !%08x in user-facing display) but did not retro-apply it, leaving 22 call sites on the bare %08x form. Those are corrected here, and a note-level trunk linter (node-id-format) now holds the line so it cannot drift again.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The pull request adds native test isolation, deterministic suite ordering, filesystem-state verdicts, expanded native coverage, runtime Portduino node-capacity validation, centralized node mute handling, and node-ID and Unity-exit linting.

Changes

Native testing infrastructure

Layer / File(s)	Summary
Filesystem state isolation and classification bin/lib/test-state.sh, bin/pio-test-isolate.sh, test/TestUtil.*, variants/native/portduino/platformio.ini, bin/test-state-check.sh	Native suites use isolated homes. Filesystem changes and surviving processes are recorded and classified.
Deterministic ordering and verdict reporting .github/workflows/test_native.yml, bin/lib/shuffle.sh, bin/run-tests.sh, test/README.md, AGENTS.md	The workflow and runner support seeded ordering, replay commands, state-aware verdicts, preserved diagnostics, and sanitizer reporting.
Native suite coverage and termination test/test_admin_radio/test_main.cpp, test/test_default/test_main.cpp, test/test_fscommon_getfiles/test_main.cpp, test/test_*/**, test/native-suite-count	Tests cover node metadata, traffic scaling, filesystem enumeration, and explicit Unity process termination. The canonical suite count is 44.

Platform configuration and diagnostics

Layer / File(s)	Summary
Runtime capacity and configuration includes src/mesh/mesh-pb-constants.h, src/mesh/NodeDB.h, src/platform/portduino/ConfigCheck.cpp, docs/node_info_stores.md, src/**	Portduino capacity comes from configuration. NodeDB retains a separate 250-node migration ceiling and validates excessive MaxNodes values. Configuration headers precede dependent headers.
Node formatting and menu behavior src/graphics/draw/MenuHandler.*, src/mesh/PacketHistory.cpp, src/modules/*.cpp, test/test_admin_radio/test_main.cpp	Node mute changes use toggleNodeMuted(). Node and packet identifiers use consistent hexadecimal formatting.
Node-ID and Unity-exit linting bin/lint-node-id-format.sh, bin/lint-unity-exit.sh, .github/node-id-format-allowlist.txt, .trunk/trunk.yaml, bin/test-lint-unity-exit.sh	Non-blocking linters detect bare node-ID formats and bare UNITY_END() calls. Self-tests cover Unity-exit detection.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• meshtastic/firmware#10593: Both changes modify and test FSCommon::getFiles().
• meshtastic/firmware#11130: Both changes modify native workflow orchestration and suite ordering.
• meshtastic/firmware#11224: Both changes update General.MaxNodes validation in ConfigCheck.cpp.

Suggested labels: cleanup, github_actions

Suggested reviewers: thebentern

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.83% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately identifies the main change as a rebuild of the native test suite, although it omits specific implementation details.
Description check	✅ Passed	The description clearly covers motivation, implementation phases, results, exclusions, and testing status, including the required device-testing limitation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.

[Copilot]

🟡 Not ready to approve

Two small correctness issues were found in new code paths (log ID formatting in MenuHandler::toggleNodeMuted() and regex-based MISSING detection in bin/lib/test-state.sh) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR hardens the native/portduino unit-test harness against persisted $HOME state leakage by isolating each suite in its own scratch home, reporting undeclared filesystem writes as an outcome axis, and adding targeted regression/characterization coverage (FS manifest bounds, admin node-metadata saves, and menu mute toggle). It also fixes a portduino-specific build hazard where MAX_NUM_NODES could silently differ per translation unit when mesh-pb-constants.h was included without configuration.h.

Changes:

• Add per-suite $HOME isolation via a PlatformIO test_testing_command wrapper, plus shared-state reporting (CLEAN/DIRTY/MISSING) driven by a central test/state-manifest.tsv.
• Rebuild/expand native coverage: new test_fscommon_getfiles suite, new Default::getConfiguredOrDefaultMsScaled(..., TrafficType) tests, and per-test fixture isolation + new admin/menu metadata tests in test_admin_radio.
• Enforce correct portduino node-cap semantics by #error-guarding the ARCH_PORTDUINO branch in mesh-pb-constants.h and fixing include order in affected translation units; extract menuHandler::toggleNodeMuted() for testability.

File summaries

File	Description
variants/native/portduino/platformio.ini	Registers per-suite isolation wrapper for native and coverage test envs.
test/TestUtil.h	Redefines RUN_TEST to checkpoint test-state after each Unity test.
test/TestUtil.cpp	Adds optional per-test filesystem change attribution (portduino-only).
test/test_fscommon_getfiles/test_main.cpp	New suite covering bounded getFiles() behavior and capacity-release idiom.
test/test_default/test_main.cpp	Adds tests for region-throttle overload of getConfiguredOrDefaultMsScaled.
test/test_admin_radio/test_main.cpp	Makes globals fixture per-test; adds admin metadata + menu mute toggle tests.
test/state-manifest.tsv	Central allowlist of deliberate per-suite persisted writes + reasons.
test/README.md	Updates guidance: isolation, CLEAN/DIRTY, shuffling, and failure interpretation.
test/native-suite-count	Bumps canonical suite count (43 → 44).
src/SerialConsole.cpp	Ensures configuration.h is included before headers relying on portduino caps.
src/platform/esp32/MeshtasticOTA.cpp	Same include-order fix for portduino cap correctness.
src/mqtt/ServiceEnvelope.cpp	Same include-order fix for portduino cap correctness.
src/mesh/StreamAPI.cpp	Same include-order fix for portduino cap correctness.
src/mesh/raspihttp/PiWebServer.cpp	Same include-order fix for portduino cap correctness (in portduino-only TU).
src/mesh/NodeDB.h	Names NODEDB_MIGRATION_LOAD_CEILING and clarifies it is not the runtime cap.
src/mesh/mesh-pb-constants.h	#error if ARCH_PORTDUINO reaches a compile-time MAX_NUM_NODES definition.
src/mesh/api/ServerAPI.cpp	Same include-order fix for portduino cap correctness.
src/mesh/api/PacketAPI.cpp	Same include-order fix for portduino cap correctness.
src/graphics/draw/MenuHandler.h	Exposes toggleNodeMuted() for test reachability.
src/graphics/draw/MenuHandler.cpp	Extracts mute toggle logic into toggleNodeMuted().
docs/node_info_stores.md	Updates documentation to reflect native/portduino runtime node cap semantics.
bin/test-state-check.sh	Adds self-test fixtures for the shared-state checker.
bin/run-tests.sh	Adds shared-state grading, suite shuffling, seed replay, and manifest proposals.
bin/pio-test-isolate.sh	New wrapper: per-suite scratch $HOME + leftover-state classification/retention.
bin/lib/test-state.sh	New shared helpers for fingerprinting/classifying suite filesystem changes.
AGENTS.md	Updates native test-failure debugging guidance (CLEAN/DIRTY axis, signals, shuffle).
.github/workflows/test_native.yml	Adds deterministic CI area-order shuffle with replayable seed controls.
.github/copilot-instructions.md	Documents new isolation/state-reporting model and portduino cap semantics.

Review details

• Files reviewed: 24/28 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (2)

src/mesh/mesh-pb-constants.h (1)

115-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the new C++ comments.

The new explanations exceed the repository's two-line limit and repeat the same runtime-cap rationale. Keep each comment to one or two lines. Move detailed background to docs/node_info_stores.md if needed.

• src/mesh/mesh-pb-constants.h#L115-L123: reduce the include-order explanation to one or two lines and retain only why configuration.h must precede this header.
• src/mesh/NodeDB.h#L24-L29: reduce the migration-ceiling explanation to one or two lines.
• src/mesh/NodeDB.h#L528-L533: reduce the load-ceiling calculation explanation to one or two lines.

As per coding guidelines, keep C++ comments minimal and avoid restating code behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/mesh-pb-constants.h` around lines 115 - 123, Shorten the comments at
src/mesh/mesh-pb-constants.h:115-123, src/mesh/NodeDB.h:24-29, and
src/mesh/NodeDB.h:528-533 to one or two lines each. In the mesh-pb-constants.h
comment, retain only that configuration.h must be included first for the runtime
MAX_NUM_NODES definition; in NodeDB.h, retain only the essential
migration-ceiling and load-ceiling calculation context. Remove repeated
rationale and move any necessary background to docs/node_info_stores.md.

Source: Coding guidelines

test/TestUtil.cpp (1)

69-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider std::filesystem instead of manual dirent/stat recursion.

fileFingerprint and walk reimplement a recursive directory walk with opendir/readdir/lstat. Since this code only compiles under ARCH_PORTDUINO (host builds), std::filesystem::recursive_directory_iterator and std::filesystem::file_size/hashing over an ifstream would remove the manual pointer management and work uniformly across the Linux/macOS/Windows native targets this project already builds for.

This is optional: the current code is correct as written.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/TestUtil.cpp` around lines 69 - 107, Optionally replace the manual POSIX
traversal in walk with std::filesystem::recursive_directory_iterator, preserving
relative regular-file keys and skipping directories or inaccessible entries as
the current implementation does. Update fileFingerprint to use standard C++ file
I/O, retaining the existing hash algorithm and return behavior for unreadable
files; limit the change to the host-only ARCH_PORTDUINO code path.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test_native.yml:
- Around line 262-265: Update the seed initialization in the workflow step
around seed_input so the suite_order_seed input is passed through the step’s env
configuration rather than interpolated into the shell script. Read that
environment variable as a quoted shell value, preserving the existing
empty-check and explicit-seed behavior while preventing shell metacharacters
from being executed.

In `@bin/pio-test-isolate.sh`:
- Around line 82-95: Replace the `paste -sd'; ' -` join in the `PER_TEST_DETAIL`
assignment with a delimiter-safe approach that separates every listed undeclared
path using the exact `"; "` sequence. Preserve the existing five-item truncation
and additional-count suffix behavior.

In `@bin/run-tests.sh`:
- Around line 216-230: Eliminate the duplicated MINSTD Fisher-Yates
implementation by extracting shuffle_suites into a shared helper while
preserving bin/run-tests.sh as the canonical behavior. Update bin/run-tests.sh
at lines 216-230 to source/invoke that helper, and replace the inline awk block
in .github/workflows/test_native.yml at lines 275-286 with the same shared
helper call so both paths remain byte-identical.

In `@bin/test-state-check.sh`:
- Around line 19-22: Update the directory setup in test-state-check.sh so the cd
to ROOT_DIR is guarded and the script exits immediately with a clear error if
changing directories fails. Preserve the existing SCRIPT_DIR and ROOT_DIR
resolution.

In `@docs/node_info_stores.md`:
- Around line 64-66: Update the capacity documentation in
docs/node_info_stores.md so the property matrix includes the ESP32-S3 100-node
tier or explicitly identifies its values as representative, while preserving
consistency with the platform table. Document that the host default is 200 nodes
and that wasm_config_apply() overrides portduino_config.MaxNodes to 80 for WASM,
including the affected footnote or platform notes.

In `@src/graphics/draw/MenuHandler.cpp`:
- Around line 2934-2947: Update the LOG_INFO call in
menuHandler::toggleNodeMuted to format nodeNum as a lowercase, zero-padded
hexadecimal identifier with the required 0x prefix, using the `0x%08x` format.

In `@src/mesh/NodeDB.h`:
- Around line 532-533: Update the MaxNodes validation before the size_t
conversion used by getMaxNodesAllocatedSize and the loadCeiling calculation:
clamp or reject positive values above the intended runtime maximum so they
cannot become an oversized MAX_NUM_NODES value. Preserve the existing handling
for nonpositive values and ensure loadCeiling is computed only from the bounded
node count.

---

Nitpick comments:
In `@src/mesh/mesh-pb-constants.h`:
- Around line 115-123: Shorten the comments at
src/mesh/mesh-pb-constants.h:115-123, src/mesh/NodeDB.h:24-29, and
src/mesh/NodeDB.h:528-533 to one or two lines each. In the mesh-pb-constants.h
comment, retain only that configuration.h must be included first for the runtime
MAX_NUM_NODES definition; in NodeDB.h, retain only the essential
migration-ceiling and load-ceiling calculation context. Remove repeated
rationale and move any necessary background to docs/node_info_stores.md.

In `@test/TestUtil.cpp`:
- Around line 69-107: Optionally replace the manual POSIX traversal in walk with
std::filesystem::recursive_directory_iterator, preserving relative regular-file
keys and skipping directories or inaccessible entries as the current
implementation does. Update fileFingerprint to use standard C++ file I/O,
retaining the existing hash algorithm and return behavior for unreadable files;
limit the change to the host-only ARCH_PORTDUINO code path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 93a54d1c-46fe-4d41-a289-95de317310d3

📥 Commits

Reviewing files that changed from the base of the PR and between d0e9d02 and a797bc4.

⛔ Files ignored due to path filters (1)

• test/state-manifest.tsv is excluded by !**/*.tsv

📒 Files selected for processing (27)

• .github/copilot-instructions.md
• .github/workflows/test_native.yml
• AGENTS.md
• bin/lib/test-state.sh
• bin/pio-test-isolate.sh
• bin/run-tests.sh
• bin/test-state-check.sh
• docs/node_info_stores.md
• src/SerialConsole.cpp
• src/graphics/draw/MenuHandler.cpp
• src/graphics/draw/MenuHandler.h
• src/mesh/NodeDB.h
• src/mesh/StreamAPI.cpp
• src/mesh/api/PacketAPI.cpp
• src/mesh/api/ServerAPI.cpp
• src/mesh/mesh-pb-constants.h
• src/mesh/raspihttp/PiWebServer.cpp
• src/mqtt/ServiceEnvelope.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• test/README.md
• test/TestUtil.cpp
• test/TestUtil.h
• test/native-suite-count
• test/test_admin_radio/test_main.cpp
• test/test_default/test_main.cpp
• test/test_fscommon_getfiles/test_main.cpp
• variants/native/portduino/platformio.ini

[Copilot]

🟡 Not ready to approve

The new shared-state fingerprinting and suite discovery use GNU-only tooling (find -printf, md5sum), which is not available by default on macOS despite the repo shipping a native-macos environment, so the harness should be made portable (or explicitly gated) before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 30/36 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

bin/lint-node-id-format.sh (1)

53-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Tighten looks_like_id()'s broad keyword fallback.

The last four alternatives in looks_like_id() match any occurrence of "packet", "node", "sender", or "relay" anywhere in the accumulated statement. In files like PacketHistory.cpp, every log message is prefixed with "Packet History - ...", so this fallback matches almost every statement, regardless of whether the specific %08x argument is actually an ID.

This contradicts the design rationale stated in the header comment: a false positive costs more than a miss. Tighten the fallback to match ID-shaped tokens near the message, not the presence of a generic domain word.

♻️ Proposed tightening

 	function looks_like_id(s) {
 		return (s ~ /(->|\.)(num|from|to|id|dest|sender|relay_node|next_hop)[^A-Za-z0-9_]/) ||
 		       (s ~ /[Nn]ode[Nn]um/) || (s ~ /nodeId/) || (s ~ /getFrom[ \t]*\(/) ||
-		       (s ~ /[^A-Za-z0-9_]sender[^A-Za-z0-9_]/) ||
-		       (s ~ /[Nn]ode/) || (s ~ /[Pp]acket/) || (s ~ /[Ss]ender/) || (s ~ /[Rr]elay/)
+		       (s ~ /[^A-Za-z0-9_]sender[^A-Za-z0-9_]/) ||
+		       (s ~ /relay_?[Nn]ode/) || (s ~ /relayed_by/) || (s ~ /ourRelayID/)
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/lint-node-id-format.sh` around lines 53 - 58, Update the final broad
keyword alternatives in looks_like_id() so packet, node, sender, and relay only
trigger when they occur in an ID-shaped token or relevant identifier context
near the message, rather than anywhere in the accumulated statement. Preserve
the existing specific patterns and the header comment’s preference for avoiding
false positives.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/lib/shuffle.sh`:
- Around line 15-18: Update shuffle_suites to return immediately when $# is
zero, before invoking printf, so an empty suite list produces no blank output;
preserve the existing shuffling behavior when suite names are provided.

In `@bin/run-tests.sh`:
- Around line 280-291: Update verdict_red() to aggregate the “test cases:”
summaries from every shuffled suite in $LOG when $SHUFFLE is set, rather than
selecting only the final matching line. Preserve the existing last-line behavior
for non-shuffled runs and ensure the RED report reflects failures from all suite
invocations.

---

Nitpick comments:
In `@bin/lint-node-id-format.sh`:
- Around line 53-58: Update the final broad keyword alternatives in
looks_like_id() so packet, node, sender, and relay only trigger when they occur
in an ID-shaped token or relevant identifier context near the message, rather
than anywhere in the accumulated statement. Preserve the existing specific
patterns and the header comment’s preference for avoiding false positives.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 71fdce5b-bafc-40fa-b796-c8196b756e38

📥 Commits

Reviewing files that changed from the base of the PR and between a797bc4 and ffe460f.

📒 Files selected for processing (16)

• .github/node-id-format-allowlist.txt
• .github/workflows/test_native.yml
• .trunk/trunk.yaml
• bin/lib/shuffle.sh
• bin/lib/test-state.sh
• bin/lint-node-id-format.sh
• bin/pio-test-isolate.sh
• bin/run-tests.sh
• bin/test-state-check.sh
• docs/node_info_stores.md
• src/graphics/draw/MenuHandler.cpp
• src/mesh/PacketHistory.cpp
• src/mesh/mesh-pb-constants.h
• src/modules/NodeInfoModule.cpp
• src/modules/PositionModule.cpp
• src/platform/portduino/ConfigCheck.cpp

🚧 Files skipped from review as they are similar to previous changes (6)

• bin/test-state-check.sh
• docs/node_info_stores.md
• bin/pio-test-isolate.sh
• .github/workflows/test_native.yml
• bin/lib/test-state.sh
• src/graphics/draw/MenuHandler.cpp

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.trunk/trunk.yaml:
- Around line 82-94: Update bin/lint-unity-exit.sh so its UNITY_END() scanning
preserves /* ... */ block-comment state across physical lines and ignores
occurrences inside comments. Accumulate and analyze continued multi-line calls,
including cases where UNITY_END() appears within an existing exit( invocation,
so valid wrapped calls are not reported. Enable the unity-exit rule in
trunk.yaml only after the scanner handles these statement-aware cases.

In `@bin/lint-unity-exit.sh`:
- Around line 36-59: The Unity scanner around bare_unity_end and its diagnostic
loop must become stateful and match-aware. Track multi-line comment and
string-literal state across AWK records, recognize logical statements spanning
lines such as split exit(UNITY_END()), and preserve detection of
whitespace-separated bare calls without matching ignored text. Update reporting
to emit one diagnostic for each actual match at its recorded column rather than
once per physical line with a fixed column, and add fixtures covering these
cases.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d8edbb0c-f71b-49a7-b19e-bc0ce351e6d2

📥 Commits

Reviewing files that changed from the base of the PR and between ffe460f and 2a4f9cc.

📒 Files selected for processing (13)

• .github/copilot-instructions.md
• .trunk/trunk.yaml
• AGENTS.md
• bin/lib/shuffle.sh
• bin/lib/test-state.sh
• bin/lint-unity-exit.sh
• bin/pio-test-isolate.sh
• bin/run-tests.sh
• bin/test-state-check.sh
• test/README.md
• test/test_meshpacket_serializer/test_serializer.cpp
• test/test_mqtt/MQTT.cpp
• test/test_serial/SerialModule.cpp

🚧 Files skipped from review as they are similar to previous changes (4)

• AGENTS.md
• bin/pio-test-isolate.sh
• bin/run-tests.sh
• .github/copilot-instructions.md

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

bin/run-tests.sh (1)

443-443: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Aggregate the failure count in the RED fallback line for shuffled runs.

This line takes tail -1 of every "[0-9]+ failed" match in $LOG. In shuffled mode, each suite appends its own pio test output to the same $LOG (Lines 290-303), so this reproduces the exact bug already fixed for the "test cases:" summary via summarise_test_cases() (Lines 366-390): if an earlier suite fails and a later suite also fails with a different count, this machine-readable RESULT: line reports only the last matching count, not the true total. This directly works against the PR's stated goal of clarifying Unity failure-count reporting.

Use the same aggregation approach as summarise_test_cases() here.

🐛 Proposed fix

-	echo "RESULT: RED $(grep -oE '[0-9]+ failed' "$LOG" | tail -1 || echo 'build/crash error')"
+	total_failed=$(grep -oE '[0-9]+ failed' "$LOG" | awk '{s+=$1} END{print s+0}')
+	if ((total_failed > 0)); then
+		echo "RESULT: RED $total_failed failed"
+	else
+		echo "RESULT: RED build/crash error"
+	fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/run-tests.sh` at line 443, Update the RED result reporting around the
existing RESULT line to aggregate all failure counts from LOG, matching the
summation behavior implemented by summarise_test_cases(). Preserve the existing
build/crash error fallback when no failure count is found, and ensure shuffled
runs report the total failures across all suites rather than only the final
match.

🧹 Nitpick comments (3)

test/TestUtil.cpp (1)

140-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Add direct coverage for the new checkpoint/diff logic.

testStateCheckpoint, walk, fileFingerprint, and suiteFromPath are new, non-trivial logic (recursive traversal, FNV-1a hashing, add/modified/removed diffing, suite-name parsing). No test in the provided files exercises this C++ path directly; bin/test-state-check.sh only covers the separate shell-side state_classify logic. Consider a small dedicated suite that sets MESHTASTIC_TEST_STATE_REPORT/HOME to a scratch directory, calls testStateCheckpoint across synthetic file changes, and asserts the emitted added/modified/removed lines.

As per path instructions, test/**/*: Add or update tests for behavior changes when suites are added or removed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/TestUtil.cpp` around lines 140 - 184, Add direct C++ tests for the
checkpoint logic centered on testStateCheckpoint, using a scratch HOME and
MESHTASTIC_TEST_STATE_REPORT environment, synthetic files, and multiple calls to
verify added, modified, and removed report lines. Also cover suiteFromPath
parsing and the walk/fileFingerprint behavior through these checkpoints, and
place or update tests under the applicable test suite so added or removed suites
are handled.

Source: Path instructions

bin/lint-unity-exit.sh (1)

29-30: 🩺 Stability & Availability | 🔵 Trivial

Do not report scanner failures as clean scans.

The shell ignores the awk exit status, and Line 110 always returns 0. A read or parser failure can therefore produce no diagnostic while satisfying Trunk's success_codes: [0].

Keep findings non-blocking, but emit a Trunk-formatted diagnostic for scanner failures or expose a separate failure status. This contract is defined by .trunk/trunk.yaml Lines 82-94.

Also applies to: 107-110

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/lint-unity-exit.sh` around lines 29 - 30, Update the scanner flow in the
shell script so read or awk/parser failures are not treated as clean scans.
Preserve non-blocking findings, but propagate scanner failure status or emit a
Trunk-formatted diagnostic consistent with the configured success-code contract,
and ensure the script’s final status reflects scanner failures instead of always
returning success.

test/test_fscommon_getfiles/test_main.cpp (1)

1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Trim comments to the guideline's one-to-two-line limit.

Three comment blocks in this file exceed the repository's comment-length rule: the file header (Lines 1-8, 7 comment lines), the FSCom-path rationale (Lines 20-22, 3 lines), and the capacity-release rationale (Lines 202-205, 4 lines). The content is useful context, but the guideline caps comments at one or two lines and asks for minimal, non-restating comments.

Condense each block to its core rationale in one or two lines.

As per path instructions, "Keep code comments minimal—one or two lines maximum—and comment only when explaining non-obvious rationale; do not restate straightforward code."

Also applies to: 20-22, 202-205

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test_fscommon_getfiles/test_main.cpp` around lines 1 - 8, Condense the
file header comment to one or two lines summarizing the getFiles() coverage and
bounded-walk rationale. Also shorten the FSCom-path rationale and
capacity-release rationale comments to one or two lines each, preserving only
their non-obvious context and removing behavior restatements.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/lint-unity-exit.sh`:
- Around line 44-59: Rewrite strip_comments as a stateful lexical scanner that
recognizes escaped string literals, character literals, and raw-string literals,
preserving their contents so tokens such as UNITY_END() and /* do not affect
comment state. Replace the greedy block-comment match with one-pair-at-a-time
scanning while maintaining in_block across lines, and add fixtures covering both
false positives inside literals and missed real calls after literal or
block-comment content.
- Around line 74-79: Update bare_unity_end() so wrapper exemptions are
token-aware: only recognize a standalone exit(UNITY_END()) and capture-then-exit
usage, excluding identifiers such as myexit and operators such as ==, !=, >=,
and +=. Remove the return UNITY_END() exemption, since return does not terminate
the runner, while preserving detection of other bare UNITY_END() calls.

In `@bin/run-tests.sh`:
- Around line 290-303: Add "${PASSTHRU[@]}" to both PlatformIO test invocations
inside the shuffled loop in the SHUFFLE path, covering the QUIET and tee
branches. Preserve the existing suite filter, logging, and return-code handling
while ensuring all collected user flags are forwarded for every suite.
- Around line 78-80: Update the startup directory change in bin/run-tests.sh by
guarding the cd "$ROOT_DIR" command so failure terminates the script
immediately; preserve the existing ROOT_DIR calculation and ensure later
relative-path operations cannot run from an unintended working directory.

In `@bin/test-state-check.sh`:
- Around line 91-130: Update the survivor PID lookup in the test block around
the `survivor_dir` and `leaked_pid` checks so its glob matches the fixture’s
`$HOME/../survivor.pid` location under the sandbox home directory. Preserve the
existing verification that the reported survivor is actually reaped, or
alternatively change the fixture’s PID output location to match the current
lookup.

---

Outside diff comments:
In `@bin/run-tests.sh`:
- Line 443: Update the RED result reporting around the existing RESULT line to
aggregate all failure counts from LOG, matching the summation behavior
implemented by summarise_test_cases(). Preserve the existing build/crash error
fallback when no failure count is found, and ensure shuffled runs report the
total failures across all suites rather than only the final match.

---

Nitpick comments:
In `@bin/lint-unity-exit.sh`:
- Around line 29-30: Update the scanner flow in the shell script so read or
awk/parser failures are not treated as clean scans. Preserve non-blocking
findings, but propagate scanner failure status or emit a Trunk-formatted
diagnostic consistent with the configured success-code contract, and ensure the
script’s final status reflects scanner failures instead of always returning
success.

In `@test/test_fscommon_getfiles/test_main.cpp`:
- Around line 1-8: Condense the file header comment to one or two lines
summarizing the getFiles() coverage and bounded-walk rationale. Also shorten the
FSCom-path rationale and capacity-release rationale comments to one or two lines
each, preserving only their non-obvious context and removing behavior
restatements.

In `@test/TestUtil.cpp`:
- Around line 140-184: Add direct C++ tests for the checkpoint logic centered on
testStateCheckpoint, using a scratch HOME and MESHTASTIC_TEST_STATE_REPORT
environment, synthetic files, and multiple calls to verify added, modified, and
removed report lines. Also cover suiteFromPath parsing and the
walk/fileFingerprint behavior through these checkpoints, and place or update
tests under the applicable test suite so added or removed suites are handled.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fa394e84-30b0-40be-86cb-fdb0255a12e5

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4f9cc and 92624a0.

⛔ Files ignored due to path filters (1)

• test/state-manifest.tsv is excluded by !**/*.tsv

📒 Files selected for processing (39)

• .github/copilot-instructions.md
• .github/node-id-format-allowlist.txt
• .github/workflows/test_native.yml
• .trunk/trunk.yaml
• AGENTS.md
• bin/lib/shuffle.sh
• bin/lib/test-state.sh
• bin/lint-node-id-format.sh
• bin/lint-unity-exit.sh
• bin/pio-test-isolate.sh
• bin/run-tests.sh
• bin/test-state-check.sh
• docs/node_info_stores.md
• src/SerialConsole.cpp
• src/graphics/draw/MenuHandler.cpp
• src/graphics/draw/MenuHandler.h
• src/mesh/NodeDB.h
• src/mesh/PacketHistory.cpp
• src/mesh/StreamAPI.cpp
• src/mesh/api/PacketAPI.cpp
• src/mesh/api/ServerAPI.cpp
• src/mesh/mesh-pb-constants.h
• src/mesh/raspihttp/PiWebServer.cpp
• src/modules/NodeInfoModule.cpp
• src/modules/PositionModule.cpp
• src/mqtt/ServiceEnvelope.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• src/platform/portduino/ConfigCheck.cpp
• test/README.md
• test/TestUtil.cpp
• test/TestUtil.h
• test/native-suite-count
• test/test_admin_radio/test_main.cpp
• test/test_default/test_main.cpp
• test/test_fscommon_getfiles/test_main.cpp
• test/test_meshpacket_serializer/test_serializer.cpp
• test/test_mqtt/MQTT.cpp
• test/test_serial/SerialModule.cpp
• variants/native/portduino/platformio.ini

🚧 Files skipped from review as they are similar to previous changes (31)

• src/modules/PositionModule.cpp
• .trunk/trunk.yaml
• src/modules/NodeInfoModule.cpp
• src/mesh/raspihttp/PiWebServer.cpp
• .github/node-id-format-allowlist.txt
• src/graphics/draw/MenuHandler.h
• src/mesh/api/ServerAPI.cpp
• variants/native/portduino/platformio.ini
• src/platform/portduino/ConfigCheck.cpp
• bin/lib/shuffle.sh
• src/mqtt/ServiceEnvelope.cpp
• src/mesh/api/PacketAPI.cpp
• src/SerialConsole.cpp
• src/mesh/PacketHistory.cpp
• test/test_meshpacket_serializer/test_serializer.cpp
• src/mesh/NodeDB.h
• test/test_default/test_main.cpp
• test/test_serial/SerialModule.cpp
• src/mesh/mesh-pb-constants.h
• AGENTS.md
• test/test_mqtt/MQTT.cpp
• .github/copilot-instructions.md
• test/test_admin_radio/test_main.cpp
• src/platform/esp32/MeshtasticOTA.cpp
• bin/lint-node-id-format.sh
• test/TestUtil.h
• src/mesh/StreamAPI.cpp
• test/native-suite-count
• docs/node_info_stores.md
• .github/workflows/test_native.yml
• src/graphics/draw/MenuHandler.cpp

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

bin/lint-unity-exit.sh (1)

109-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore the per-occurrence diagnostic contract. The scanner records one macro position and emits one result per accumulated statement. The self-test only checks whether output exists, so it cannot prevent this regression.

• bin/lint-unity-exit.sh#L109-L126: evaluate each UNITY_END() occurrence independently and emit its own line and column when it is bare.
• bin/test-lint-unity-exit.sh#L31-L47: assert exact finding counts and locations, including mixed legal and bare calls in one statement.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/lint-unity-exit.sh` around lines 109 - 126, Update the statement scanner
around the UNITY_END detection in bin/lint-unity-exit.sh (lines 109-126) to
evaluate every UNITY_END() occurrence independently and emit its own line and
column only when that occurrence is bare, including mixed legal and bare calls
within one statement. Strengthen the self-test in bin/test-lint-unity-exit.sh
(lines 31-47) to assert exact finding counts and reported locations rather than
merely checking that output exists.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/test-lint-unity-exit.sh`:
- Around line 31-47: Enhance the expect() helper in the lint test to accept and
validate the expected diagnostic count and location details, including line and
column, rather than only checking whether output exists. Update existing cases
with those expectations and add a fixture covering exit(UNITY_END());
UNITY_END(); on one line, asserting both diagnostics and their locations.

---

Outside diff comments:
In `@bin/lint-unity-exit.sh`:
- Around line 109-126: Update the statement scanner around the UNITY_END
detection in bin/lint-unity-exit.sh (lines 109-126) to evaluate every
UNITY_END() occurrence independently and emit its own line and column only when
that occurrence is bare, including mixed legal and bare calls within one
statement. Strengthen the self-test in bin/test-lint-unity-exit.sh (lines 31-47)
to assert exact finding counts and reported locations rather than merely
checking that output exists.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bf0640f2-298a-47df-bec2-a0c742e53ad1

📥 Commits

Reviewing files that changed from the base of the PR and between 92624a0 and 7a4aead.

📒 Files selected for processing (4)

• bin/lint-unity-exit.sh
• bin/run-tests.sh
• bin/test-lint-unity-exit.sh
• bin/test-state-check.sh

🚧 Files skipped from review as they are similar to previous changes (2)

• bin/test-state-check.sh
• bin/run-tests.sh

[Copilot]

🟡 Human review recommended

It makes broad, workflow-integrated changes to the test harness/CI execution model that warrant a final human validation pass in CI to confirm PlatformIO runner behavior and end-to-end compatibility.

Review details

• Files reviewed: 33/41 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.