Implement rotating JSONL recorder for persistent logging (#10428)

* Implement rotating JSONL recorder for persistent logging * Fixes * Update documentation and clean up imports in command files * Address remaining recorder review feedback Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/2541773c-869a-463f-9fae-8505272c06ff Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * recorder: fix lock re-entry deadlock on start() and force_rotate_all() The previous "Fixes" commit added `_files_snapshot()` which acquires `self._lock` so handlers don't race with `stop()` clearing `_files`. But two callers were already holding `self._lock` when they invoked methods that go through the snapshot: - `start()` writes the `recorder_start` event from inside its `with self._lock:` block. `_write_event` -> `_files_snapshot` re-acquires the same non-reentrant `threading.Lock`, freezing process startup. - `force_rotate_all()` calls `self.status()` (which also acquires `self._lock`) while still holding the lock from rotating each file. Both fixes release the lock before the call. The recorder_start marker still lands in events.jsonl because the started/started_at flags are already set when we write it. Verified end-to-end against the standalone /tmp/verify_pr_fixes.py harness — all 9 PR review-comment fixes pass, including pause/resume event ordering and concurrent start/stop without KeyError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix markdown linting issues in leakhunt.md and repro.md * Handle recorder startup and query review fixes Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Tighten recorder follow-up tests Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Stabilize recorder startup tests Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Remove brittle recorder startup test Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Polish recorder follow-up errors Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Refine recorder startup and regex errors Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Clean up recorder follow-up nits Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/78540a9f-fe62-4350-b252-0ae5621f0b8a Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com> * Trunk --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 09:22:40 -05:00
parent 10a7f1042b
commit f6a954b97e
17 changed files with 3047 additions and 5 deletions
@@ -0,0 +1,88 @@
+"""Unit tests for the `build_flags` injection on `flash.build()`.
+
+We don't actually run pio here — too slow, requires hardware-aware envs.
+We test the translation layer (`_build_flags_env`) and that the env vars
+are threaded through pio.run correctly via mock.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import patch
+
+from meshtastic_mcp import flash, pio
+
+
+class TestBuildFlagsEnv:
+    def test_simple_value(self) -> None:
+        out = flash._build_flags_env({"DEBUG_HEAP": 1})
+        assert out == {"PLATFORMIO_BUILD_FLAGS": "-DDEBUG_HEAP=1"}
+
+    def test_string_value(self) -> None:
+        out = flash._build_flags_env({"FOO": "bar"})
+        assert out == {"PLATFORMIO_BUILD_FLAGS": "-DFOO=bar"}
+
+    def test_bool_true_is_bare_flag(self) -> None:
+        out = flash._build_flags_env({"DEBUG_HEAP": True})
+        assert out == {"PLATFORMIO_BUILD_FLAGS": "-DDEBUG_HEAP"}
+
+    def test_bool_false_dropped(self) -> None:
+        out = flash._build_flags_env({"DEBUG_HEAP": False, "OTHER": 1})
+        assert out == {"PLATFORMIO_BUILD_FLAGS": "-DOTHER=1"}
+
+    def test_none_dropped(self) -> None:
+        out = flash._build_flags_env({"DEBUG_HEAP": None})
+        assert out == {}
+
+    def test_multiple_combined(self) -> None:
+        out = flash._build_flags_env({"DEBUG_HEAP": 1, "FOO": "x", "BAR": True})
+        # Order isn't guaranteed in dict iteration, so check membership.
+        flags = out["PLATFORMIO_BUILD_FLAGS"].split()
+        assert set(flags) == {"-DDEBUG_HEAP=1", "-DFOO=x", "-DBAR"}
+
+
+class TestBuildPropagatesFlags:
+    def test_extra_env_passed_to_pio_run(self) -> None:
+        # Mock pio.run so we don't actually invoke pio. Capture extra_env.
+        captured = {}
+
+        class _StubResult:
+            returncode = 0
+            stdout = ""
+            stderr = ""
+            duration_s = 0.1
+
+        def _stub(args, **kwargs):
+            captured["args"] = args
+            captured["kwargs"] = kwargs
+            return _StubResult()
+
+        with patch.object(pio, "run", side_effect=_stub):
+            with patch.object(flash, "_artifacts_for", return_value=[]):
+                out = flash.build(
+                    "fake-env",
+                    with_manifest=False,
+                    build_flags={"DEBUG_HEAP": 1},
+                )
+        assert captured["args"] == ["run", "-e", "fake-env"]
+        assert captured["kwargs"]["extra_env"] == {
+            "PLATFORMIO_BUILD_FLAGS": "-DDEBUG_HEAP=1"
+        }
+        assert out["build_flags"] == {"DEBUG_HEAP": 1}
+
+    def test_no_flags_means_no_extra_env(self) -> None:
+        captured = {}
+
+        class _StubResult:
+            returncode = 0
+            stdout = ""
+            stderr = ""
+            duration_s = 0.1
+
+        def _stub(args, **kwargs):
+            captured["kwargs"] = kwargs
+            return _StubResult()
+
+        with patch.object(pio, "run", side_effect=_stub):
+            with patch.object(flash, "_artifacts_for", return_value=[]):
+                flash.build("fake-env", with_manifest=False)
+        assert captured["kwargs"]["extra_env"] is None