test(meshcore): Phase 2 — companion channel + repeater DM auto-ack messaging system-test

[Yeraze]

Summary

Phase 2 of the MeshCore system-tests epic — adds messaging assertions to
tests/test-meshcore.sh (the harness from Phase 1, PR #4232). No product code
changes; test infrastructure only. No system-tests.sh change needed — the
existing "MeshCore Hardware Test" phase already runs this script.

What it verifies

1. Companion↔companion channel messaging (hard) — creates a dedicated
   #mm-systest hashtag channel on both companions (PSK auto-derived:
   base64(SHA-256("#mm-systest")[:16])), sends a channel message A→B, and
   verifies B receives it.
2. Yeraze Repeater DM auto-ack (hard) — resolves the repeater contact by
   name / advType==2, DMs it, and catches the firmware auto-ack
   (meshcore:send-confirmed) via a small Socket.IO listener helper
   (tests/helpers/meshcore-await-ack.mjs, using the repo's own
   socket.io-client). Delivery ACK is Socket.IO-only — never exposed over REST.
3. Companion↔companion DM (opt-in, MESHCORE_COMPANION_DM=1, default SKIP).

Why companion↔companion DM is opt-in (near-field RF)

Extensive live-hardware diagnosis (documented in
docs/internal/dev-notes/MESHCORE_SYSTEM_TESTS_EPIC.md): the two reference
companions sit ~3 inches apart, and a direct DM+ACK round-trip between them
fails every configuration — before and after establishing contacts/paths via a
flood-advert exchange, over both a repeater-relayed (pathLen=1) and a direct
(pathLen=0) path. Meanwhile channel broadcasts between the same two nodes
work fine (~3s, both directions) and the repeater DM auto-acks (~492ms).
Broadcasts getting through while the DM+ACK round-trip doesn't is the signature
of near-field receiver overload — a physical layout limitation, not a
software/test bug. So the companion↔companion DM is gated behind
MESHCORE_COMPANION_DM (default skip, with a clear SKIP message); when enabled it
first runs an advert-exchange + contact-refresh and tries both directions
(inbox OR ack), for use once the nodes are physically separated.

Validation

Every shipping assertion was validated independently on the live rig this
session: channel A→B delivered in ~3s (both directions), repeater DM auto-ack in
~492ms, connect/handshake repeatedly green, companion-DM skip is deterministic.
Full Vitest suite green (3157 suites, 0 failures — test-only changes, outside
Vitest's collection glob); npm run typecheck clean; bash -n + node --check
pass.

Operational note

The MeshCore system-test needs exclusive access to the companion serial ports
(ttyUSB) — locally and on the CI hw runner. Nothing else may hold a MeshCore
serial connection to those ports while the system-test-labeled job runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Phase 2 MeshCore System Tests

Tasks:

• Gather context and understand the PR
• Read changed files
• Analyze code quality, bugs, and security
• Post review feedback

---

Overall Assessment

This is a well-structured test-infrastructure-only PR. The approach is pragmatic (round-robin port probing, opt-in DM gate, Socket.IO ack listener for firmware events that aren't REST-visible), and the live-hardware validation rationale is clearly documented. A few issues worth addressing before merge:

---

Issues

1. connect event fires twice — READY printed twice, timer cleared twice (meshcore-await-ack.mjs:43–49)

socket.on('connect', () => { console.log('READY'); });          // line 43
// ...
socket.on('connect', () => clearTimeout(connectDeadline));      // line 49

Two separate connect listeners are registered. If the socket reconnects (which is enabled — reconnection: true), both fire again: the bash script receives a second READY line in $ACK_OUT, and the deadline timer is re-cleared (harmless, but noisy). More critically, if reconnection happens after the bash sender already started, the bash grep -q READY logic is fine, but the extra READY line leaks into the file used by grep '^ACK ' which could confuse future pattern matching.

Fix: Merge the two listeners:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
});

2. Timeout fires TIMEOUT_MS + 12000 ms from script start, not from READY (meshcore-await-ack.mjs:51)

setTimeout(() => { ... finish(1); }, TIMEOUT_MS + 12000);

The 12s padding accounts for connection time, but the effective budget is slightly unpredictable: if connection takes 1s the real ack budget is TIMEOUT_MS + 11s; if it takes 11s it's TIMEOUT_MS + 1s. Since bash already waits for READY before sending, the cleaner approach is to start the ack-wait timer on the connect event:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
  setTimeout(() => { console.error(`no ack within ${TIMEOUT_MS}ms`); finish(1); }, TIMEOUT_MS);
});

This is a minor correctness nit, not a blocking issue.

3. sourceId comparison is string vs. potential number (meshcore-await-ack.mjs:38)

if (data && data.sourceId && data.sourceId !== SOURCE_ID) return;

SOURCE_ID is a process env string. If data.sourceId is ever emitted as a number (e.g. integer DB row id), !== will reject it. Consider String(data.sourceId) !== SOURCE_ID for robustness.

4. Channel slot selection iterates 1–7 but MeshCore may support more or fewer (test-meshcore.sh:585)

for c in 1 2 3 4 5 6 7; do echo "$USED" | grep -qx "$c" || { SLOT=$c; break; }; done

The magic range 1–7 has no comment linking it to the MeshCore firmware's actual channel count limit. If a future firmware revision supports 8+ channels (or only 4), this silently fails or over-claims. Add a comment or make it an env override like the other limits.

5. dm_all_400=true logic inverted for channel test (test-meshcore.sh:622–644)

chan_all_400=true
for attempt in $(seq 1 "$CHAN_ATTEMPTS"); do
  ...
  [ "$SEND_CODE" = "400" ] || chan_all_400=false

This logic is correct (same pattern as the DM test). Just flagging it to confirm it was intentional — chan_all_400 stays true only if every send was 400. ✓

6. Temporary file /tmp/mc_send.$$ not cleaned up in Test 10/11 path (test-meshcore.sh:623, 680)

rm -f "$ACK_OUT" "$ACK_ERR" "/tmp/mc_send.$$" 2>/dev/null

The cleanup() function removes /tmp/mc_send.$$ correctly. But during Test 11 (channel send loop), each iteration writes to /tmp/mc_send.$$ and the file persists across loop iterations. If Test 12 fails mid-loop after writing to /tmp/mc_send.$$, and a subsequent signal triggers cleanup() before Test 12's write, the file is cleaned. This is fine — just confirming the cleanup coverage is correct. ✓

7. HELPER_PID kill race in Test 9 (companion DM) vs Test 12 (repeater) (test-meshcore.sh:549, 683)

In Test 9 (companion DM loop), HELPER_PID is assigned per pair-iteration and killed+cleared after each pair:

kill "$HELPER_PID" 2>/dev/null; wait "$HELPER_PID" 2>/dev/null
ACK_OUT=""; ACK_ERR=""; HELPER_PID=""

In Test 12 (repeater), HELPER_PID is assigned once and waited at the end (line 683). If a signal arrives between the last DM attempt and wait $HELPER_PID, cleanup() will kill "$HELPER_PID" and wait will return immediately. This is correct. ✓

8. seq 1 15 busy-poll for READY may race with ack (test-meshcore.sh:521, 668)

for _ in $(seq 1 15); do grep -q READY "$ACK_OUT" && break; sleep 1; done

If an ack arrives extremely fast (within the 15-second READY wait), grep '^ACK ' on line 539/674 will still see it because the file is append-only. The for _ in $(seq 1 15) correctly just waits for READY before sending, not before checking for acks. This is fine. ✓

9. node "$(dirname "$0")/helpers/meshcore-await-ack.mjs" — dirname may not resolve correctly in all CI environments (test-meshcore.sh:519, 664)

When bash ./tests/test-meshcore.sh is run from the repo root, $0 is the full path and dirname "$0" works. But if the script is sourced or run with bash -c "...", $0 could be bash. This is a common pattern in the repo's other test scripts, so it's likely consistent — just worth noting.

10. SLOT comparison uses string vs integer in jq (test-meshcore.sh:602)

jq -r --arg slot "$SLOT" '(.data // .)[] | select((.id|tostring)==$slot) | .name // empty'

The |tostring coercion handles both "id": 2 and "id": "2" cases. ✓

---

Minor Style/Robustness Notes

• TEST 8 API token request (test-meshcore.sh:471) doesn't capture/check the HTTP status code from the curl -X POST, only checking jq -r '.token // empty'. A 401/403 response body might accidentally contain a token field in error scenarios. Recommend adding -w "\n%{http_code}" and checking 200.
• sleep 25 / sleep 5 in companion DM advert exchange (lines 496, 500): these are hardcoded waits. They work, but a comment linking these to the MeshCore flood-advert propagation timing would help future maintainers calibrate if the rig changes.
• reconnection: true in meshcore-await-ack.mjs (line 29): Reconnection is correct for resilience, but since the helper is single-use (exits after first ack or timeout), reconnection can cause the socket to reconnect after socket.close() is called in finish(). The try { socket.close() } handles this gracefully.

---

Summary

The code is functional and well-reasoned. The two actionable bugs are:

1. Duplicate connect listener in meshcore-await-ack.mjs — merge into one
2. sourceId strict-equality type mismatch potential — use String() coercion

Everything else is nits or confirmed-correct. The test design (opt-in companion DM, advert pre-step, both-direction retry, Socket.IO ack listener as the canonical delivery signal) is solid and the hardware rationale is well-documented.

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Phase 2 MeshCore System Tests

Tasks:

• Gather context and understand the PR
• Read changed files
• Analyze code quality, bugs, and security
• Post review feedback

---

Overall Assessment

Solid test-infrastructure-only PR. The approach (opt-in companion DM gate, Socket.IO ack listener for firmware events not exposed over REST, live-hardware near-field rationale) is well-reasoned and clearly documented. A couple of real bugs worth fixing before merge.

---

Bugs

1. Duplicate connect listener — READY printed on reconnect, deadline cleared twice (meshcore-await-ack.mjs:43,49)

socket.on('connect', () => { console.log('READY'); });   // line 43
// ...
socket.on('connect', () => clearTimeout(connectDeadline)); // line 49

Two separate connect handlers are registered. Since reconnection: true (line 29), if the socket reconnects after the initial connection (e.g., transient network blip during a long TIMEOUT_MS wait), both fire: the bash receiver gets a second READY line appended to $ACK_OUT, and the deadline is re-cleared. The second READY is benign for grep -q READY, but leaks into the same file used by grep '^ACK ' — a future pattern regression risk.

Fix — merge the two handlers:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
});

2. sourceId type mismatch — strict !== against env string (meshcore-await-ack.mjs:38)

if (data && data.sourceId && data.sourceId !== SOURCE_ID) return;

SOURCE_ID is a string from process.env. If the Socket.IO server emits data.sourceId as a number (integer DB row ID), !== will always evaluate false and the filter will never match, silently passing all acks from all sources through. This is easy to miss in happy-path testing because on the current rig there's only one sender.

Fix:

if (data && data.sourceId && String(data.sourceId) !== SOURCE_ID) return;

---

Correctness Notes (non-blocking)

3. Ack timeout budget counted from process start, not from READY (meshcore-await-ack.mjs:51)

setTimeout(() => { ... finish(1); }, TIMEOUT_MS + 12000);

The +12000 accounts for connection time, but if the socket connects in 1s, the real ack budget is TIMEOUT_MS + 11s; if it takes 11s to connect, the budget is TIMEOUT_MS + 1s. Since bash already waits for READY before sending, starting the ack timer on connect would give a deterministic TIMEOUT_MS ack window:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
  setTimeout(() => { console.error(`no ack within ${TIMEOUT_MS}ms`); finish(1); }, TIMEOUT_MS);
});

Minor predictability nit, not a gate blocker.

4. Channel slot range 1–7 has no comment linking to firmware limit (test-meshcore.sh:585)

for c in 1 2 3 4 5 6 7; do echo "$USED" | grep -qx "$c" || { SLOT=$c; break; }; done

The range is a magic number. MESHCORE_TEST_CHANNEL_SLOT override exists and is used, which is good. A short comment (e.g., # MeshCore firmware supports slots 1-7) would make the limit auditable when the firmware changes.

5. READY poll loop doesn't check if helper exited early (test-meshcore.sh:521,668)

for _ in $(seq 1 15); do grep -q READY "$ACK_OUT" && break; sleep 1; done

Test 12 does check after the loop (line 669): grep -q READY "$ACK_OUT" || { ... kill $HELPER_PID; exit 1; }. Test 9's companion DM path does not have this guard — if the helper exits with code 2 (connect error) before printing READY, the loop exhausts its 15 iterations and the bash script proceeds to send DMs with no listener, misreporting a delivery failure as an RF issue rather than a test-infra issue. Low risk on the current rig, but worth adding parity with Test 12's guard:

grep -q READY "$ACK_OUT" || { echo -e "${RED}✗ FAIL${NC}: helper never READY (companion DM $SENDER->$RECEIVER)"; cat "$ACK_ERR"; kill "$HELPER_PID" 2>/dev/null; exit 1; }

6. Token endpoint: HTTP status not checked, only body shape (test-meshcore.sh:470–472)

API_TOKEN=$(curl -s -X POST "$BASE_URL/api/token/generate" \
    -b "$COOKIE_FILE" -H "x-csrf-token: $CSRF_TOKEN" | jq -r '.token // empty')
[ -n "$API_TOKEN" ] || { echo -e "${RED}✗ FAIL${NC}: no API token"; exit 1; }

A 401/403 response with an error body containing no token field correctly fails the [ -n "$API_TOKEN" ] check. But a hypothetical JSON error that accidentally included a token key would pass. Recommend -w "\n%{http_code}" + status check. Low-severity given the test context.

---

Style / Minor Notes

• sleep 25 / sleep 5 in companion DM advert exchange (lines 496, 500): These are hardcoded; a comment linking them to MeshCore flood-advert propagation timing would help future maintainers calibrate if the rig changes.
• reconnection: true in meshcore-await-ack.mjs:29: Correct for resilience. try { socket.close() } in finish() (line 34) handles the reconnect-after-close case gracefully. ✓
• HELPER_PID cleanup in Test 9 vs Test 12: Test 9 kills+clears HELPER_PID per pair-iteration (line 549–550); Test 12 waits (line 683) and lets cleanup() handle signal races. Both patterns are correct. ✓
• Ack fast-path race: If an ack arrives before bash sends (extremely unlikely), grep '^ACK ' on the per-pair check still catches it because $ACK_OUT is append-only. ✓

---

Summary

Two actionable bugs:

1. Duplicate connect listener in meshcore-await-ack.mjs — merge into one (lines 43, 49)
2. sourceId strict-equality type mismatch — String() coerce before !== (line 38)

Everything else is nits or confirmed-correct logic. The test design (opt-in companion DM, both-direction retry, Socket.IO listener as the canonical ack signal, near-field rationale documented) is pragmatic and solid.

[github-actions]

System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 20:52:41 EDT

❌ Overall Result: FAILED

Aborted on first failing test (fail-fast mode). Remaining tests skipped.

Failed Test

• MeshCore Hardware Test

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Phase 2 MeshCore System Tests

Tasks:

• Gather context and understand the PR
• Read changed files
• Analyze code quality, bugs, and security
• Post review feedback

---

Overall Assessment

Solid test-infrastructure-only PR. The architecture (Socket.IO ack listener for firmware events not exposed over REST, opt-in companion DM gate with clear near-field rationale, round-robin port probing) is well-designed. The two bugs flagged in earlier reviews remain unfixed — they're worth addressing before merge.

---

Bugs (still present)

1. Duplicate connect listener — READY printed on reconnect (meshcore-await-ack.mjs:43,49)

socket.on('connect', () => { console.log('READY'); });     // line 43
// ...
socket.on('connect', () => clearTimeout(connectDeadline)); // line 49

Two separate connect handlers are registered. With reconnection: true (line 29), if the socket reconnects during a long TIMEOUT_MS wait, both fire: a second READY line is appended to $ACK_OUT, polluting the file used by grep '^ACK ' later. The deadline is also cleared twice (harmless, but noisy). The fix is simple — merge into one handler:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
});

2. sourceId strict !== against env string — silent filter bypass (meshcore-await-ack.mjs:38)

if (data && data.sourceId && data.sourceId !== SOURCE_ID) return;

SOURCE_ID is a process.env string. If the Socket.IO server emits data.sourceId as a number (integer DB row ID), the !== comparison will always be false and the guard will never filter, allowing acks from all sources through. On a single-node test rig this is invisible in happy-path runs, but on a multi-source rig it could produce false positives.

if (data && data.sourceId && String(data.sourceId) !== SOURCE_ID) return;

---

Non-blocking Issues

3. Ack timeout is from process start, not from READY (meshcore-await-ack.mjs:51)

setTimeout(() => { ... finish(1); }, TIMEOUT_MS + 12000);

The +12000 approximates connection time, but the effective ack budget varies with actual connection latency. Since bash already waits for READY before sending, starting the timer on connect would give a deterministic TIMEOUT_MS window. As-is, a slow-connecting socket slightly shortchanges the ack budget.

4. Missing READY guard in Test 9 companion DM path (test-meshcore.sh:521)

Test 12 (repeater) has a READY guard:

grep -q READY "$ACK_OUT" || { echo -e "${RED}✗ FAIL${NC}: helper never became READY"; cat "$ACK_ERR"; kill $HELPER_PID 2>/dev/null; exit 1; }

Test 9 (companion DM) only polls with for _ in $(seq 1 15); do ... done — if the helper exits early (connect error, exit 2) before printing READY, the loop exhausts its 15 iterations and the script proceeds to send DMs with no listener. The resulting failure is then misattributed to RF loss rather than a test-infrastructure error. Adding the same guard (consistent with Test 12) would surface infra failures distinctly.

5. Token endpoint — HTTP status not checked (test-meshcore.sh:470–472)

API_TOKEN=$(curl -s -X POST "$BASE_URL/api/token/generate" \
    -b "$COOKIE_FILE" -H "x-csrf-token: $CSRF_TOKEN" | jq -r '.token // empty')
[ -n "$API_TOKEN" ] || { echo -e "${RED}✗ FAIL${NC}: no API token"; exit 1; }

A 401/403 response without a token key correctly fails the emptiness check. Low-severity given the test context, but -w "\n%{http_code}" + explicit status check would match the pattern used in Tests 2/3.

6. Magic range 1–7 for channel slot search (test-meshcore.sh:588)

for c in 1 2 3 4 5 6 7; do echo "$USED" | grep -qx "$c" || { SLOT=$c; break; }; done

The range is a magic number with no comment linking it to the MeshCore firmware's actual channel count. MESHCORE_TEST_CHANNEL_SLOT override is already provided (good). Adding a comment like # MeshCore firmware: slots 1–7 makes the limit auditable when firmware changes.

7. sleep 25 / sleep 5 in companion DM advert exchange (test-meshcore.sh:496,500)

These hardcoded waits are calibrated for the reference rig. A comment linking them to MeshCore flood-advert propagation timing (and the fact they may need adjustment if rig topology changes) would help future maintainers.

---

Confirmed Correct

• dm_all_400 / chan_all_400 / rpt_all_400 logic: correctly stays true only if every send returns 400, cleanly distinguishing "target not a contact" from RF delivery failure. ✓
• HELPER_PID cleanup: Test 9 kills+clears per-pair; Test 12 waits and lets cleanup() handle signal races. Both patterns are correct. ✓
• |tostring coercion in select((.id|tostring)==$slot): handles both integer and string id values from the API. ✓
• Ack fast-path: if an ack arrives before the send, $ACK_OUT is append-only and grep '^ACK ' still catches it. ✓
• try { socket.close() } in finish(): gracefully handles reconnect-after-close. ✓
• Dual-path message check (per-channel route + inbox fallback): sensible resilience for API shape variance. ✓

---

Summary

Two bugs to fix before merge (items 1 and 2 above — both in meshcore-await-ack.mjs). Everything else is non-blocking nits. The test design is pragmatic and the hardware rationale is well-documented.

[github-actions]

System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 21:13:06 EDT

❌ Overall Result: FAILED

Aborted on first failing test (fail-fast mode). Remaining tests skipped.

Failed Test

• MeshCore Hardware Test

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Phase 2 MeshCore System Tests

Tasks:

• Gather context and understand the PR
• Read changed files
• Analyze code quality, bugs, and security
• Post review feedback

---

Overall Assessment

Well-structured test-infrastructure-only PR. The Socket.IO ack-listener design, the opt-in companion DM gate, and the pre/post-clean for the #mm-systest channel are all pragmatic and well-reasoned. Two real bugs need fixing before merge; everything else is nits.

---

Bugs

1. Duplicate connect listener — READY printed twice on reconnect (meshcore-await-ack.mjs:43,49)

socket.on('connect', () => { console.log('READY'); });     // line 43
// ...
socket.on('connect', () => clearTimeout(connectDeadline)); // line 49

With reconnection: true (line 29), both listeners fire on every reconnect. The second READY line is appended to $ACK_OUT, the same file polled by grep '^ACK ' on lines 539/674 — not a current problem but a future regression risk. The connectDeadline is cleared twice (harmless but noisy).

Fix — merge into one:

socket.on('connect', () => {
  console.log('READY');
  clearTimeout(connectDeadline);
});

2. sourceId strict !== against env string — silent filter bypass (meshcore-await-ack.mjs:38)

if (data && data.sourceId && data.sourceId !== SOURCE_ID) return;

SOURCE_ID is a process.env string. If the Socket.IO server emits data.sourceId as a number (integer DB row ID from a JSON-serialized Drizzle row), the strict !== is always false and the guard never fires — all acks from all sources pass through. On the current single-sender rig this is invisible in happy-path testing.

Fix:

if (data && data.sourceId && String(data.sourceId) !== SOURCE_ID) return;

---

Non-Blocking Issues

3. Ack timeout budget measured from process start, not from READY (meshcore-await-ack.mjs:51)

setTimeout(() => { ... finish(1); }, TIMEOUT_MS + 12000);

The +12000 pads for connection time, but if connection takes 1s the real ack window is TIMEOUT_MS + 11s; if it takes 11s the window is TIMEOUT_MS + 1s. Since bash already gates on READY before sending, starting the timer inside the connect handler would give a deterministic TIMEOUT_MS ack window. Minor predictability issue, not a gate blocker.

4. Missing READY guard in Test 9 companion DM path (test-meshcore.sh:521)

Test 12 (repeater) has:

grep -q READY "$ACK_OUT" || { echo -e "${RED}✗ FAIL${NC}: helper never became READY"; cat "$ACK_ERR"; kill $HELPER_PID 2>/dev/null; exit 1; }

Test 9 (companion DM) only has the polling loop (line 521) with no post-loop guard. If the helper exits early (exit 2 / connect error) before printing READY, the 15-iteration loop exhausts and the script proceeds to send DMs with no listener. The resulting timeout failure is then misattributed to RF loss rather than a test-infra error. Adding the same guard as Test 12 would surface infra failures distinctly.

5. Token acquisition doesn't check HTTP status (test-meshcore.sh:470–472)

API_TOKEN=$(curl -s -X POST "$BASE_URL/api/token/generate" \
    -b "$COOKIE_FILE" -H "x-csrf-token: $CSRF_TOKEN" | jq -r '.token // empty')
[ -n "$API_TOKEN" ] || { echo -e "${RED}✗ FAIL${NC}: no API token"; exit 1; }

A 401/403 response without a token key correctly fails the emptiness check. Low-severity given this is a test harness, but consistent with the pattern used by Tests 2/3 which capture -w "\n%{http_code}" separately.

6. Magic slot range 1–7 has no comment linking to firmware limit (test-meshcore.sh:606)

for c in 1 2 3 4 5 6 7; do echo "$USED" | grep -qx "$c" || { SLOT=$c; break; }; done

No comment explains why 7 is the upper bound. The MESHCORE_TEST_CHANNEL_SLOT env override is already provided (good). A short comment like # MeshCore firmware: channel slots 1–7 makes the limit auditable when firmware changes.

---

Confirmed Correct

• dm_all_400 / chan_all_400 / rpt_all_400 logic: correctly stays true only if every send returned 400 in that test block, cleanly distinguishing "target not a contact" from RF delivery failure. ✓
• Pre/post-clean for #mm-systest channel: the pre-clean before Test 10 and post-clean after Test 11 correctly handle the case where a prior test run persisted the channel to hardware — without the pre-clean, two matching slots would cause MeshCore to route to the first one, silently breaking the new send. ✓
• jq type-safe slot selection (|tostring coercion in select((.id|tostring)==$slot)): handles both integer and string id values from the API. ✓
• HELPER_PID cleanup: Test 9 kills+clears per pair-iteration (line 549–550); Test 12 waits (line 713) and lets cleanup() handle signal races. Both patterns are correct. ✓
• Ack fast-path: if an ack arrives before bash sends, $ACK_OUT is append-only and grep '^ACK ' still catches it. ✓
• try { socket.close() } in finish(): gracefully handles reconnect-after-close since socket.close() with reconnection enabled can itself trigger a reconnect on some socket.io versions. ✓
• Round-robin port probing in Test 5: polling all candidate ports in a single loop rather than sequential blocking per-port correctly avoids burning the full LIVE_MAX_WAIT budget on a dead port before checking live ones. ✓
• Both-direction retry in Test 9: checking either the receiver inbox OR a sender-side ack correctly handles firmware ack races and contact directionality variance across rigs. ✓

---

Summary

Two bugs to fix (both in meshcore-await-ack.mjs):

1. Merge the duplicate connect listeners (lines 43, 49)
2. String() coerce data.sourceId before !== comparison (line 38)

Everything else is confirmed-correct or minor nits. The test design (Socket.IO ack listener as the canonical delivery signal for events not exposed over REST, opt-in companion DM gate with hardware rationale, both-direction retry, channel pre/post-clean for hardware persistence) is solid and well-documented.

[github-actions]

System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 21:39:22 EDT

Test Summary

Test Suite	Result
Configuration Import	✅ PASSED
Quick Start Test	✅ PASSED
Security Test	✅ PASSED
V1 API Test	✅ PASSED
Reverse Proxy Test	✅ PASSED
Reverse Proxy + OIDC	✅ PASSED
Virtual Node CLI Test	✅ PASSED
Backup & Restore Test	✅ PASSED
Database Migration Test	✅ PASSED
DB Backing Consistency	✅ PASSED
API Exercise (3 DBs)	✅ PASSED
MeshCore Hardware Test	✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

• Tests configuration import and device reboot cycle
• Verifies channel roles, PSKs, and LoRa configuration
• Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

• Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
• HTTP access without HSTS
• Auto-generated admin user with default credentials
• Session cookies work over HTTP
• Meshtastic node connection and message exchange verified

Security Test:

• Verifies Node IP address hidden from anonymous users in API responses
• Verifies MQTT configuration hidden from anonymous users
• Verifies Node IP address visible to authenticated users
• Verifies MQTT configuration visible to authenticated users
• Verifies protected endpoints require authentication

V1 API Test:

• Tests v1 REST API endpoints with Bearer token authentication
• Verifies Bearer token requests bypass CSRF protection
• Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
• Verifies session-based requests still require CSRF token

Reverse Proxy Test:

• Production deployment with COOKIE_SECURE=true
• HTTPS-ready configuration
• Trust proxy enabled for reverse proxy compatibility
• CORS configured for HTTPS domain
• Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

• OIDC authentication integration
• Mock OIDC provider health checks
• Authorization flow and session creation
• Hybrid mode (OIDC + local auth)
• Meshtastic node connection verified

Virtual Node CLI Test:

• Virtual Node Server enabled on TCP port 4404
• Meshtastic Python client successfully connects
• Node data download and synchronization verified
• Test message sent on gauntlet channel (index 3)
• Message delivery confirmed via Web UI API
• Virtual Node Server connection logging verified

Backup & Restore Test:

• System backup created from running dev container
• New container spun up with RESTORE_FROM_BACKUP env var
• Data integrity verified (node count, message count, settings)
• Restore event logged in audit log
• Dev container unaffected by restore test

Database Migration Test:

• SQLite to PostgreSQL migration verified
• SQLite to MySQL migration verified
• Data integrity confirmed for both target databases
• Row counts match between source and target

DB Backing Consistency Test:

• SQLite, PostgreSQL, and MySQL backends tested with same device
• Node counts within ±10 across all three backends
• Favorite counts identical across all backends
• Key station verified as favorite on all backends

[Yeraze]

Thanks for the thorough review. Addressed in fcd2ce3:

• "Claude PR Assistant workflow" #1 + feat: traceroute highlighting and UI improvements #2 (double READY / timeout window): merged the two connect listeners behind a connectedOnce guard, so READY prints exactly once and the ack-wait timer starts from the first connect (stable TIMEOUT_MS rather than TIMEOUT_MS + 12s from process start). Also fixes the reconnect double-fire.
• "Update Claude PR Assistant workflow" #3 (sourceId type): now String(data.sourceId) !== SOURCE_ID.
• feat: traceroute request tracking and auth documentation #4 (slot range): commented the 1-7 scan (slot 0 = Public; companions expose 0-7) and noted the existing MESHCORE_TEST_CHANNEL_SLOT override.
• Minor note (Test 8 token): now captures %{http_code} and fails on non-200.

Items #5-#10 and the reconnection:true note were confirmations that the code is correct as-is — left unchanged. The sleep 25/5 advert-exchange waits only run under the opt-in MESHCORE_COMPANION_DM path; they're calibrated to MeshCore flood-advert propagation (documented in the epic doc's near-field finding).

Note: this is a behavior-preserving refactor of test-only code; the hardware system-test re-runs on this commit to re-validate.

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: Phase 2 MeshCore System Tests

Tasks:

• Gather context and understand the PR
• Read changed files (current commit fcd2ce3)
• Verify previously flagged bugs are fixed
• Analyze for remaining issues
• Post review feedback

---

Overall Assessment

All bugs from the previous review rounds are fixed in fcd2ce3. System tests are passing (all 12 suites green including MeshCore Hardware Test). This is clean, well-structured test-infrastructure code — ready to merge.

---

Previously Flagged Bugs: Resolved ✓

1. Duplicate connect listener — fixed (meshcore-await-ack.mjs:53–59)

The two separate socket.on('connect', ...) handlers are now merged behind a connectedOnce guard:

let connectedOnce = false;
socket.on('connect', () => {
  if (connectedOnce) return;
  connectedOnce = true;
  console.log('READY');
  clearTimeout(connectDeadline);
  setTimeout(() => { console.error(`no ack within ${TIMEOUT_MS}ms`); finish(1); }, TIMEOUT_MS);
});

This simultaneously fixes:

• READY printing exactly once (no duplicate on reconnect)
• The ack-wait timer starting from connect rather than process start (deterministic TIMEOUT_MS budget)
• The deadline being cleared once, not twice

All three non-blocking issues from the prior reviews (#1, #2, #3) resolved in one clean change. ✓

2. sourceId type mismatch — fixed (meshcore-await-ack.mjs:39)

if (data && data.sourceId && String(data.sourceId) !== SOURCE_ID) return;

String() coercion added. ✓

3. Token endpoint HTTP status — fixed (test-meshcore.sh:470–476)

TOKEN_RESP=$(curl -s -w "\n%{http_code}" -X POST "$BASE_URL/api/token/generate" ...)
TOKEN_CODE=$(echo "$TOKEN_RESP" | tail -n1)
API_TOKEN=$(echo "$TOKEN_RESP" | head -n-1 | jq -r '.token // empty')
if [ "$TOKEN_CODE" != "200" ] || [ -z "$API_TOKEN" ]; then

HTTP status is now captured and checked explicitly. ✓

4. Channel slot range comment — fixed (test-meshcore.sh:610–613)

# Scan slots 1-7 (slot 0 is Public/reserved; MeshCore companions expose 8
# channel slots 0-7) for the first free one on both sources. Override with
# MESHCORE_TEST_CHANNEL_SLOT if a future firmware changes the channel count.
for c in 1 2 3 4 5 6 7; do ...

Comment added linking the 1–7 range to firmware semantics. ✓

---

Remaining Observations (non-blocking)

Missing READY guard in Test 9 companion DM path (test-meshcore.sh:525)

The companion DM path (opt-in under MESHCORE_COMPANION_DM=1) polls for READY but has no post-loop failure guard, unlike Test 12:

# Test 9 (no guard after loop):
for _ in $(seq 1 15); do grep -q READY "$ACK_OUT" && break; sleep 1; done

# Test 12 (has guard):
for _ in $(seq 1 15); do grep -q READY "$ACK_OUT" && break; sleep 1; done
grep -q READY "$ACK_OUT" || { echo -e "${RED}✗ FAIL${NC}: helper never became READY"; ...exit 1; }

If the helper exits early (exit 2, connect error) in Test 9, the script proceeds to send DMs with no listener and misattributes the timeout to RF loss. Since this path is opt-in and runs after advert exchange (a long, deliberate step), the risk is low — but the asymmetry with Test 12 is worth noting for the next touch.

sleep 25 / sleep 5 in companion DM advert exchange (test-meshcore.sh:500, 505)

These are calibrated for MeshCore flood-advert propagation on the reference rig. No comment links them to that timing. Low-priority, opt-in code path only.

---

Confirmed Correct

• connectedOnce guard: correctly handles reconnection: true — finish() calling socket.close() can trigger a reconnect-before-close on some socket.io versions; the done guard in finish() and connectedOnce together prevent any double-exit or double-READY. ✓
• Pre/post-clean for #mm-systest channel: the pre-clean before Test 10 correctly handles the firmware persistence issue (duplicate PSK slots break delivery); the post-clean after Test 11 is best-effort and tidy. ✓
• dm_all_400 / chan_all_400 / rpt_all_400 logic: correctly stays true only if every send in that block returned 400, giving a distinct "not-a-contact" failure path vs RF loss. ✓
• jq type-safe slot selection (|tostring coercion): handles both "id": 2 and "id": "2" from the API. ✓
• HELPER_PID cleanup: Test 9 kills+clears per pair-iteration; Test 12 waits and cleanup() handles signal races. Both correct. ✓
• Both-direction retry in Test 9: inbox OR ack success is sufficient — handles firmware ack races and contact directionality variance across rigs. ✓
• dirname "$0" for helper path: consistent with other test scripts in the repo; works correctly when run as bash ./tests/test-meshcore.sh from repo root. ✓

---

Summary

The two bugs flagged in prior reviews are fixed cleanly. The test design (Socket.IO ack listener for firmware events not exposed over REST, opt-in companion DM gate with documented near-field hardware rationale, pre/post-clean for hardware-persistent channel state, both-direction retry) is solid. Approved.

[github-actions]

System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 22:03:48 EDT

Test Summary

Test Suite	Result
Configuration Import	✅ PASSED
Quick Start Test	✅ PASSED
Security Test	✅ PASSED
V1 API Test	✅ PASSED
Reverse Proxy Test	✅ PASSED
Reverse Proxy + OIDC	✅ PASSED
Virtual Node CLI Test	✅ PASSED
Backup & Restore Test	✅ PASSED
Database Migration Test	✅ PASSED
DB Backing Consistency	✅ PASSED
API Exercise (3 DBs)	✅ PASSED
MeshCore Hardware Test	✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

• Tests configuration import and device reboot cycle
• Verifies channel roles, PSKs, and LoRa configuration
• Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

• Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
• HTTP access without HSTS
• Auto-generated admin user with default credentials
• Session cookies work over HTTP
• Meshtastic node connection and message exchange verified

Security Test:

• Verifies Node IP address hidden from anonymous users in API responses
• Verifies MQTT configuration hidden from anonymous users
• Verifies Node IP address visible to authenticated users
• Verifies MQTT configuration visible to authenticated users
• Verifies protected endpoints require authentication

V1 API Test:

• Tests v1 REST API endpoints with Bearer token authentication
• Verifies Bearer token requests bypass CSRF protection
• Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
• Verifies session-based requests still require CSRF token

Reverse Proxy Test:

• Production deployment with COOKIE_SECURE=true
• HTTPS-ready configuration
• Trust proxy enabled for reverse proxy compatibility
• CORS configured for HTTPS domain
• Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

• OIDC authentication integration
• Mock OIDC provider health checks
• Authorization flow and session creation
• Hybrid mode (OIDC + local auth)
• Meshtastic node connection verified

Virtual Node CLI Test:

• Virtual Node Server enabled on TCP port 4404
• Meshtastic Python client successfully connects
• Node data download and synchronization verified
• Test message sent on gauntlet channel (index 3)
• Message delivery confirmed via Web UI API
• Virtual Node Server connection logging verified

Backup & Restore Test:

• System backup created from running dev container
• New container spun up with RESTORE_FROM_BACKUP env var
• Data integrity verified (node count, message count, settings)
• Restore event logged in audit log
• Dev container unaffected by restore test

Database Migration Test:

• SQLite to PostgreSQL migration verified
• SQLite to MySQL migration verified
• Data integrity confirmed for both target databases
• Row counts match between source and target

DB Backing Consistency Test:

• SQLite, PostgreSQL, and MySQL backends tested with same device
• Node counts within ±10 across all three backends
• Favorite counts identical across all backends
• Key station verified as favorite on all backends