From b938b63e8a6d7918be3ef8928861044d29919598 Mon Sep 17 00:00:00 2001
From: Tom <116762865+NomDeTom@users.noreply.github.com>
Date: Sun, 14 Jun 2026 00:46:29 +0100
Subject: [PATCH] fix a long-running CI bug that overran a lot (#10707)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix the fix

* Address Copilot review: add EXIT trap and clarify PKC comment

Add `trap` to kill meshtasticd on any early exit (python harness
failure, socket timeout) so CI never leaks a background process.

Reword the ARCH_PORTDUINO comment to make explicit that pki_encrypted=true
causes the from==0 plain-admin branch to be skipped, routing into the
PKC key-check — the underlying logic was correct all along.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Update PORTDUINO comment to reflect from==0 auth fix

The from==0 branch no longer requires !pki_encrypted (fixed upstream
in this branch), so update the simulator comment to reflect the actual
remaining reason for the early intercept: is_managed could still block
exit_simulator even for local packets.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .github/workflows/test_native.yml | 22 +++++++++++++++++++++-
 src/modules/AdminModule.cpp       | 13 +++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml
index 1e22d74d1..a0af13ff3 100644
--- a/.github/workflows/test_native.yml
+++ b/.github/workflows/test_native.yml
@@ -37,13 +37,33 @@ jobs:
           sed -i -e "s#${PWD}#.#" coverage_base.info # Make paths relative.
 
       - name: Integration test
+        # Cap the whole step: if the simulator ever fails to exit (e.g. the
+        # exit_simulator admin path regresses again) the job must fail fast,
+        # not run to GitHub's 6-hour limit.
+        timeout-minutes: 5
         run: |
           .pio/build/coverage/meshtasticd -s &
           PID=$!
+          trap 'kill "$PID" 2>/dev/null || true' EXIT
           timeout 20 bash -c "until ls -al /proc/$PID/fd | grep socket; do sleep 1; done"
           echo "Simulator started, launching python test..."
           python3 -c 'from meshtastic.test import testSimulator; testSimulator()'
-          wait
+          # The Python harness sends exit_simulator and exits; the simulator is
+          # expected to terminate on its own. Give it a moment, then verify.
+          # If it is still alive the exit handshake is broken — fail loudly and
+          # do NOT fall through to `wait`, which would otherwise block until the
+          # job's hard timeout.
+          for i in $(seq 1 10); do
+            kill -0 "$PID" 2>/dev/null || break
+            sleep 1
+          done
+          if kill -0 "$PID" 2>/dev/null; then
+            echo "::error title=Simulator did not exit::meshtasticd ignored exit_simulator and is still running after the integration test. The exit_simulator admin path is broken (see AdminModule::handleReceivedProtobuf, ARCH_PORTDUINO bypass). Killing it to avoid a 6-hour CI overrun."
+            kill -9 "$PID" 2>/dev/null || true
+            wait "$PID" 2>/dev/null || true
+            exit 1
+          fi
+          wait "$PID" 2>/dev/null || true
 
       - name: Capture coverage information
         if: always() # run this step even if previous step failed
diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp
index 1511a709c..d79261d5d 100644
--- a/src/modules/AdminModule.cpp
+++ b/src/modules/AdminModule.cpp
@@ -19,6 +19,7 @@
 #include "main.h"
 #endif
 #ifdef ARCH_PORTDUINO
+#include "PortduinoGlue.h"
 #include "unistd.h"
 #endif
 
@@ -106,6 +107,18 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta
     if (mp.which_payload_variant != meshtastic_MeshPacket_decoded_tag) {
         return handled;
     }
+#ifdef ARCH_PORTDUINO
+    // Simulator only: honor exit_simulator unconditionally for the local client (from==0).
+    // The from==0 branch below now covers pki_encrypted local packets too, but is_managed
+    // can still block it. Rather than threading simulator awareness through the auth gates,
+    // intercept here before any auth logic runs. Local-origin + force_simradio only.
+    // TODO: should a local client bypass admin auth at all? Fenced to the simulator for now.
+    if (portduino_config.force_simradio && mp.from == 0 &&
+        r->which_payload_variant == meshtastic_AdminMessage_exit_simulator_tag) {
+        LOG_INFO("Exiting simulator");
+        exit(0);
+    }
+#endif
     meshtastic_Channel *ch = &channels.getByIndex(mp.channel);
     // Could tighten this up further by tracking the last public_key we went an AdminMessage request to
     // and only allowing responses from that remote.