Skip to content

test(meshcore): Phase 3 — repeater remote-admin login + telemetry poll system-test - #4248

Merged
Yeraze merged 4 commits into
mainfrom
feature/meshcore-system-tests-phase3
Jul 21, 2026
Merged

test(meshcore): Phase 3 — repeater remote-admin login + telemetry poll system-test#4248
Yeraze merged 4 commits into
mainfrom
feature/meshcore-system-tests-phase3

Conversation

@Yeraze

@Yeraze Yeraze commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 3 (final phase) of the MeshCore system-tests epic — adds remote-admin
login and remote telemetry assertions to tests/test-meshcore.sh, plus wires the
CI secret. No product code changes; test infrastructure + one workflow edit only.

What it verifies

  • Test 13 — Yeraze Repeater remote-admin login. POST .../meshcore/admin/login
    with {publicKey, password} (password from MESHCORE_REPEATER_ADMIN_PASSWORD,
    jq -R-escaped). Success = HTTP 200 + .success==true. SKIPs (exit 0) when the
    env var is unset
    (so local runs and CI-without-secret stay green); hard-fails on
    401 when set.
  • Test 14 — Remote telemetry poll. POST .../nodes/:pk/telemetry/poll
    {"type":"status"} against the repeater; success = data.written >= 1. 429-aware
    retry that honors the per-source 60s mesh-TX gate. Validated live: 16 records
    (["status:16"]).

Telemetry target — a deviation worth noting

The interview called for polling telemetry from the other companion, but that
predates Phase 2's near-field finding: remote telemetry is a request→response
round-trip to the target, so companion→companion telemetry fails the same way the
companion↔companion DM did (the two nodes are ~3in apart). The Yeraze Repeater, by
contrast, answers type:"status" (a 16-field stats blob, no login needed) and is
reachable. Per the maintainer's call, Test 14 polls the repeater only.

CI secret

Adds a step-scoped env: MESHCORE_REPEATER_ADMIN_PASSWORD: ${{ secrets.* }} to the
"Run system tests" step. If the repo secret is absent, login SKIPs and the suite
stays green — the secret must be added in repo settings for CI to exercise login.

Validation

Validated end-to-end on the live rig: connect + channel + repeater DM auto-ack +
telemetry (16 records) all pass; login SKIPs locally (no password). Full Vitest suite
green (3194 suites, 0 failures — test-only changes); typecheck clean; YAML valid.

Operational note

Like Phases 1-2, the MeshCore phase needs exclusive access to the companion serial
ports
on the CI hw runner while the system-test-labeled job runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY

Yeraze and others added 3 commits July 20, 2026 22:50
…em-tests

Phase 3 of the MeshCore system-tests epic. Appends after Test 12
(Yeraze Repeater DM auto-ack) in the same bash process, reusing the
already-resolved REPEATER_PK global plus the existing cookie/CSRF
session:

- Test 13: remote-admin login into the Yeraze Repeater via
  MESHCORE_REPEATER_ADMIN_PASSWORD. Skips (exit-0) when the secret is
  unset/empty; when set, guards REPEATER_PK is 64-hex, then hard-
  asserts HTTP 200 + success:true from POST .../meshcore/admin/login.
- Test 14: repeater-only telemetry poll (type:"status" against
  REPEATER_PK on SRC_A_ID). 429-aware retry loop honoring
  retryAfterSecs/Retry-After (expected right after Tests 12/13's mesh
  TX), hard-asserts data.written>=1 within a bounded attempt budget
  (MESHCORE_TELEMETRY_ATTEMPTS x MESHCORE_TELEMETRY_INTERVAL).
  Companion<->companion telemetry is intentionally omitted per the
  2026-07-20 near-field finding (see Phase 2's companion DM
  demotion) -- repeater-only per user decision.

Extends the end-of-run summary with login/telemetry outcome lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY
Scopes MESHCORE_REPEATER_ADMIN_PASSWORD to the "Run system tests" step
so Phase 3's Test 13 (remote-admin login) can exercise a real login on
the hardware runner once the repo secret is configured. Absent secret
keeps Test 13 at its default skip-if-unset behavior, so the suite
stays green either way.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY
…iation

Telemetry targets the Yeraze Repeater (status, 16 records validated live) rather
than the near-field-blocked other companion; login skips without the secret.
Epic complete — all three phases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY
@Yeraze Yeraze added the system-test Trigger hardware system tests label Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: Phase 3 MeshCore System Tests

Tasks:

  • Gather context and understand request
  • Read changed files
  • Review code quality and best practices
  • Check for potential bugs/issues
  • Review security considerations
  • Post final review

Overall this is clean, well-structured test infrastructure. The skip-when-unset pattern for the secret is the right call, and the 429-aware retry is solid. A few items worth flagging:

Security

jq -R . for password escaping (Test 13, line 747)

-d "{\"publicKey\":\"$REPEATER_PK\",\"password\":$(printf '%s' "$MESHCORE_REPEATER_ADMIN_PASSWORD" | jq -R .)}"

This is the correct approach — jq -R . produces a properly JSON-escaped string literal, avoiding shell injection into the JSON body. Good call here.

REPEATER_PK validation before login
The 64-hex guard at line 740–742 is appropriate:

if ! printf '%s' "$REPEATER_PK" | grep -Eiq '^[0-9a-f]{64}$'; then

This correctly prevents a malformed key from reaching the API (which would return 400 and be confusing). However, the -i flag (case-insensitive) is slightly misleading given the stored format is likely lower-hex — it's harmless but could silently accept DEADBEEF... which the API may or may not reject. Minor.

Potential Bugs / Issues

Test 14: written=0 does not retry on 200 — but the sleep interval is TELE_INTERVAL (20s) not the TX-gate wait
At line 784, when the API returns 200 but written=0, the test retries with a sleep "$TELE_INTERVAL" (20s). However, the spec notes the per-source 60s mesh-TX gate applies. If a prior telemetry request (from a previous test run that wasn't cleaned up) was very recent, the next 200-with-written=0 might happen before the gate clears, and all 5 attempts could exhaust at 20s intervals (100s total) without triggering a 429. The 429 path correctly defers; the 200-with-written=0 path doesn't. Consider using the same RA+1 logic for the non-429 retry, or documenting that written=0 means the device had no data to report (not a gate issue).

TELE_BODY temp file is created before the loop but cleaned up after (line 764, 790) — this is correct, but if the loop exits early via continue without a 200 or 429, the sleep "$TELE_INTERVAL" still fires. On an unexpected 5xx, the retry still waits 20s before the next attempt, which is reasonable behavior but not documented.

LOGIN_BODY temp file (line 743)
rm -f "$LOGIN_BODY" appears in both the pass and fail branches — good. However, it's not registered with cleanup() via ACK_OUT/ACK_ERR pattern. If the script receives a signal (e.g. SIGTERM from a timeout) between the mktemp and the rm -f, LOGIN_BODY leaks in /tmp. Given the 90-minute CI timeout this is extremely unlikely to matter in practice, but it's inconsistent with the ACK pattern.

Summary banner lines 809–810
$LOGIN_STATUS_LINE and $TELE_STATUS_LINE are both set correctly on the pass path, and LOGIN_STATUS_LINE has a default of "SKIPPED (MESHCORE_REPEATER_ADMIN_PASSWORD not set)". TELE_STATUS_LINE defaults to "FAILED" — if telemetry fails, the script exit 1s before line 810 is reached, so the default value of "FAILED" is never displayed. This is correct but could be confusing to a future reader.

Code Quality / Best Practices

429 retry uses retryAfterSecs (not retryAfterSeconds) — verify response shape
Line 773:

RA=$(jq -r '.retryAfterSecs // empty' "$TELE_BODY" 2>/dev/null); RA=${RA:-$TELE_INTERVAL}

The CLAUDE.md envelope spec says fail(res, status, code, message, extra?) where extra is passed through. The test correctly falls back to $TELE_INTERVAL if the field is absent or named differently, so this won't hard-fail — but it's worth verifying the actual field name in meshcoreRoutes.ts to avoid always falling back to the default.

.github/workflows/system-tests.yml change is minimal and correct
Adding MESHCORE_REPEATER_ADMIN_PASSWORD: ${{ secrets.MESHCORE_REPEATER_ADMIN_PASSWORD }} at step scope (not job scope) is the right level — it's only needed for one step and avoids unnecessarily exposing the secret to other steps. ✓

No product code changes — confirmed, only test infrastructure. ✓

YAML workflow indentation — the env: block at lines 69–70 is correctly scoped to the run: step. ✓

Minor Nits

  • Line 780: jq -c '.data.sources' in the pass message may print null if the field doesn't exist in the response — consider adding // "[]" to make the output cleaner.
  • Line 781: TELE_STATUS_LINE includes the jq output inline via $(jq -c ...) — this could fail silently if TELE_BODY has already been cleaned up, but it's read before rm -f so it's fine.
  • The test header comment on line 1 still says "Phase 1" only — now covers Phases 1–3.

Summary

This is solid work. The skip-when-unset guard, 429-aware retry, proper JSON escaping, and temp-file cleanup are all handled correctly. The main thing worth a follow-up (not a blocker) is clarifying whether written=0 on HTTP 200 is a TX-gate miss vs. no data, and whether the retry interval for that case should match the TX gate. Everything else is minor polish.


@github-actions

Copy link
Copy Markdown
System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 23:21:21 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

…comment, header

Non-blocking review nits (PR #4248): register LOGIN_BODY/TELE_BODY in cleanup()
(covers a signal between mktemp and rm -f); comment that HTTP 200 + written=0 is a
no-data round (not the 429 TX-gate); '.data.sources // "[]"' in the display; and
update the file header to Phases 1-3. Behavior-preserving.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158hFzsVbodeJK246qAE1dY
@Yeraze

Yeraze commented Jul 21, 2026

Copy link
Copy Markdown
Owner Author

Thanks for the thorough review. Verified + addressed in c84a44a:

  • 429 field name (your "verify the actual field name" note): confirmed correct — meshcoreRoutes.ts returns retryAfterSecs in the 429 body and sets the Retry-After header, so jq -r '.retryAfterSecs' reads the real value (no silent fallback).
  • written=0 retry: added a comment clarifying it's a no-data round (the request went out; the 60s TX-gate is the separate 429 path), so retrying at the normal interval is intentional.
  • LOGIN_BODY/TELE_BODY temp files: now declared up front and registered in cleanup() (consistent with the ACK_OUT/ACK_ERR pattern), so a signal between mktemp and rm -f no longer leaks them.
  • Minor nits: .data.sources // "[]" in the display; file header updated to Phases 1-3.
  • 64-hex -i guard: left as-is (harmless; isValidPublicKey is authoritative server-side).

All non-blocking polish — the substantive items were confirmations. Telemetry hard assertion was validated on the live rig (16 records, ["status:16"]); login skips without the secret and is exercised in CI once the repo secret is set.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: Phase 3 MeshCore System Tests (Post-Polish Pass)

Tasks:

  • Gather context and understand request
  • Read changed files (test script + workflow)
  • Check polish commit c84a44a addresses prior review items
  • Review code quality, security, bugs
  • Post final review

This is the follow-up review after the author's polish commit c84a44a addressed the prior round's feedback. The implementation is clean and well-structured. Here's the full assessment:

Prior Review Items — Resolved ✓

All substantive items from the previous review were addressed:

  • retryAfterSecs field name — confirmed correct by checking meshcoreRoutes.ts; jq reads the real value. ✓
  • written=0 retry comment — line 787–789 now documents that HTTP 200 + written=0 means an RF miss / no data (not a TX-gate issue), so the normal interval retry is intentional. ✓
  • LOGIN_BODY/TELE_BODY temp-file signal safety — both vars are now declared up front at line 469–470 and registered in cleanup() line 58 (rm -f "$ACK_OUT" "$ACK_ERR" "/tmp/mc_send.$$" "$LOGIN_BODY" "$TELE_BODY"), consistent with the ACK pattern. A SIGTERM between mktemp and rm -f no longer leaks them. ✓
  • .data.sources // "[]" nit — applied at lines 783–784. ✓
  • File header — line 2–3 now says "Phases 1-3". ✓

Current Code: Observations

Security — still correct

The jq -R . password escaping on line 750 is proper JSON-safe quoting. The 64-hex guard before login (grep -Eiq '^[0-9a-f]{64}$', line 743) is appropriate. No regressions here.

Test 13 control flow — clean

The skip-when-unset guard (line 739) is correct. The rm -f "$LOGIN_BODY" appears on both the pass path (line 760) and the fail path (line 757) before exit 1. Combined with the upfront cleanup() registration, temp files are handled on every exit path including signals.

One observation: on the exit 1 fail path, LOGIN_BODY is explicitly rm -f'd at line 757 before exit 1 — and also covered by the cleanup() trap. The double-removal is harmless (rm -f on an already-removed file is a no-op), so this is defensive-good rather than a bug.

Test 14 control flow — correct

TELE_BODY is mktemp'd at line 767 (outside the retry loop), read across all iterations, and rm -f'd at line 796 after the loop exits. The cleanup() registration is the safety net for signal interrupts. The 429-aware retry correctly defers RA+1 seconds, while the 200-with-written=0 case correctly retries at the normal TELE_INTERVAL as documented.

TELE_STATUS_LINE default value — the default is "FAILED" (line 769) but the script exit 1s at line 798 before reaching the summary banner if telemetry fails, so it's never displayed as "FAILED" in the summary. This was noted in the prior review and is still a minor readability nit — not a bug.

Workflow change — correct and minimal

Adding MESHCORE_REPEATER_ADMIN_PASSWORD: ${{ secrets.MESHCORE_REPEATER_ADMIN_PASSWORD }} scoped only to the "Run system tests" step (line 70) is the right approach — the secret isn't exposed to any other step. The skip-when-absent behavior means CI stays green even without the repo secret configured.

No product code changes — confirmed, only tests/test-meshcore.sh, the workflow YAML, and two docs files. ✓

Minor Nits (non-blocking)

  • Line 769: TELE_STATUS_LINE="FAILED" as default is slightly confusing since the FAILED value is never displayed (the exit 1 at line 798 fires first). "NOT RUN" or "DID NOT COMPLETE" would be a clearer sentinel. Optional.
  • Line 736: LOGIN_STATUS_LINE="SKIPPED (MESHCORE_REPEATER_ADMIN_PASSWORD not set)" is set unconditionally before the if guard — this correctly provides the default for the skip path. Clear and correct.

Summary

The implementation is solid. The polish commit addressed every actionable item from the previous review. The skip-when-unset guard, 429-aware retry, proper JSON escaping, and temp-file signal safety are all correct. This is ready to merge.


@github-actions

Copy link
Copy Markdown
System Test Results

MeshMonitor System Test Results

Test Run: 2026-07-20 23:44:14 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
Yeraze merged commit 1ee31e3 into main Jul 21, 2026
20 checks passed
@Yeraze
Yeraze deleted the feature/meshcore-system-tests-phase3 branch July 21, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

system-test Trigger hardware system tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant