Static analysis as reports: compiler-checked hot path, clang-tidy, lizard, clang-query, CodeQL

.clang-tidy

@@ -94,15 +94,17 @@
 #   readability-function-cognitive-complexity / *-function-size — complexity is lizard's job;
 #     it produces the per-commit number repo-health.json trends.
 #   bugprone-branch-clone — fires on deliberate parallel switch arms.
-#   clang-analyzer-security.ArrayBound — analyses the macOS SDK's headers, not our code.
+#   clang-analyzer-security.ArrayBound — analyzes the macOS SDK's headers, not our code.
+#   clang-diagnostic-function-effects — the hot-path check, owned by check_nonblocking.py
+#     and the clang-hotpath card, which splits findings by tick tier and marks what is NEW
+#     against docs/metrics/hotpath-baseline.txt. Leaving it on here double-reports the same
+#     hot-path findings with none of that context, and buries clang-tidy's own under them.
 #
 # Everything else disabled below is a style opinion we do not hold; enabling any of them
 # would gate a rewrite rather than a fix.
 #
-# NOT GATED YET. `WarningsAsErrors` stays empty until the remaining 47 clang-analyzer findings
-# are triaged (backlog: "clang-tidy: triage the 47 clang-analyzer findings"). Switching it to
-# `'*'` is the ratchet that stops a zero decaying, and it is one line — but it fails the gate
-# today, so it lands with that triage rather than before it.
+# NOT A GATE, BY DESIGN. `WarningsAsErrors` stays empty — the reasoning lives once, in
+# docs/testing.md § Static analysis, and applies to every analyser here.
 ---
 Checks: >-
   *,
@@ -197,7 +199,8 @@ Checks: >-
   -google-readability-function-size,
   -google-build-using-namespace,
   -google-runtime-int,
-  -clang-analyzer-security.ArrayBound
+  -clang-analyzer-security.ArrayBound,
+  -clang-diagnostic-function-effects
 WarningsAsErrors: ''
 HeaderFilterRegex: 'src/(core|light)/'
 FormatStyle: none

.github/codeql-config.yml

@@ -0,0 +1,11 @@
+name: "projectMM CodeQL config"
+
+# Code we did not write and will not change is not a finding we can act on. doctest.h is the
+# vendored single-header test framework (test/doctest.h) and produced the only critical alert
+# in the tree that we do not own — excluding it here is the standard mechanism, rather than
+# dismissing the same alert by hand after every scan.
+#
+# This is the ONLY vendored source in the repo; everything else under src/ and test/ is ours.
+# Add to this list only for genuinely third-party code, never to quiet a finding in our own.
+paths-ignore:
+  - test/doctest.h

.github/workflows/codeql.yml

@@ -1,37 +1,35 @@
 name: CodeQL
 
-# POC (docs/history/plans/Plan-20260727): does CodeQL's stock C/C++ suite find anything
-# real in this firmware? The question that motivated it: we parse six network packet
-# formats (ArtNet, DDP, E1.31, WLED audio sync, MQTT, WLED) plus HTTP, doing ~22 memcpy
-# operations on data arriving from the LAN, on a device with no MMU and no process
-# isolation — and we run NO security analysis at all today. Nothing else in our toolchain
-# looks for that class of bug: the compiler checks effects, lizard counts branches, our
-# Python checks read text.
+# Layer 4 of the analysis stack (docs/testing.md § Static analysis) — the only one that sees
+# whole-program taint. It earns its place on one question: we parse six network packet formats
+# (ArtNet, DDP, E1.31, WLED audio sync, MQTT, WLED) plus HTTP, doing ~22 memcpy operations on
+# data arriving from the LAN, on a device with no MMU and no process isolation. Nothing else in
+# the stack looks for that class of bug — the compiler checks effects, lizard counts branches,
+# the Python checks read text.
 #
-# `build-mode: none` (GA Oct 2025) scans C/C++ WITHOUT building it, inferring compile flags
-# per file. That matters here because our real firmware build is an ESP-IDF cross-compile
-# for Xtensa/RISC-V that a runner would have to reproduce; build-free skips that entirely.
-# The trade is some extraction noise on macro/template-dense headers — acceptable for a POC
-# whose question is "is there anything here", not "prove there is nothing".
+# It has paid for itself: 3 critical `localtime` findings in main_desktop.cpp (shared static,
+# not thread-safe), fixed via a portable isoTimestamp helper. The six packet parsers produced
+# no taint-flow findings, which is positive evidence nothing else in the stack could give.
 #
-# Free on public repos, so the cost is wall-clock only. Scheduled weekly + on demand rather
-# than per-push: this is a sweep, not a gate, until the POC says otherwise.
+# `build-mode: none` scans C/C++ WITHOUT building it, inferring compile flags per file. That
+# matters here because the real firmware build is an ESP-IDF cross-compile for Xtensa/RISC-V a
+# runner would have to reproduce. The trade is some extraction noise on macro/template-dense
+# headers.
 #
-# If this finds nothing actionable after a few runs, that is a RESULT — record it in the
-# scorecard and delete this file.
+# Free on public repos, so the cost is wall-clock only.
 
 on:
-  # Push-triggered on the POC branch, deliberately.
+  # NOT A GATE: no `pull_request` trigger, so it cannot block a merge. The reasoning is
+  # docs/testing.md § Static analysis, which every analyser here follows. The Security tab
+  # keeps the open/fixed alert lifecycle, which is the baselining we would otherwise build.
   #
-  # `workflow_dispatch` alone would NOT work here: GitHub only offers the "Run workflow"
-  # button for workflows that exist on the DEFAULT branch, and this one lives on
-  # next-iteration until the POC is judged. A push trigger is the only way to get a first
-  # result without merging an unevaluated workflow into main.
-  #
-  # This is still not a gate: it does not run on pull_request, so it cannot block a merge.
-  # It runs, we read the findings, and we decide.
+  # Both branches are listed on purpose. A workflow whose trigger names a branch nobody is
+  # pushing to runs never, and reports nothing while looking healthy — this job sat idle for
+  # two commits when the working branch was called `next` instead of `next-iteration`
+  # (docs/history/lessons.md). `main` keeps it firing once this branch is merged and retired.
   push:
     branches:
+      - main
       - next-iteration
     paths:
       # Only when C/C++ or the workflow itself changes — a docs commit has nothing new to
@@ -70,6 +68,8 @@ jobs:
           # question above, and the quality ones tell us whether CodeQL sees anything our
           # existing checks miss. Narrow it later if the noise is not worth it.
           queries: security-and-quality
+          # Excludes vendored code (test/doctest.h) — see the file for why.
+          config-file: ./.github/codeql-config.yml
 
       - name: Perform CodeQL analysis
         uses: github/codeql-action/analyze@v3

.github/workflows/test.yml

@@ -80,20 +80,46 @@ jobs:
     strategy:
       fail-fast: false          # a TSan race and an ASan UAF are independent signals — report both
       matrix:
-        kind: [address, thread]
+        # `realtime` is the runtime half of the hot-path check: MM_NONBLOCKING marks the tick
+        # methods, -Wfunction-effects proves what it can at compile time, and RTSan catches what
+        # it cannot — allocation or blocking reached through virtual dispatch or a function
+        # pointer. Same attribute drives both, so this lane costs one matrix entry.
+        kind: [address, thread, realtime]
     steps:
       - uses: actions/checkout@v4
         with:
           persist-credentials: false
       # The CMake configure needs uv on PATH — it resolves UV_EXECUTABLE for the UI-embed step
       # (CLAUDE.md § Use uv for every Python invocation), so a build without it fails at configure.
       - uses: astral-sh/setup-uv@v3
+      # RTSan needs Clang 20+; ubuntu-latest ships Clang 18, which rejects
+      # `-fsanitize=realtime` at the compiler-probe stage. Install a new enough one for that
+      # lane only — ASan/TSan run on the runner's default compiler.
+      - name: install clang for RealtimeSanitizer
+        if: matrix.kind == 'realtime'
+        run: |
+          wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | sudo tee /etc/apt/trusted.gpg.d/llvm.asc
+          sudo add-apt-repository -y "deb http://apt.llvm.org/$(lsb_release -cs)/ llvm-toolchain-$(lsb_release -cs)-20 main"
+          sudo apt-get install -y clang-20
       - name: build + run unit tests under ${{ matrix.kind }}sanitizer
         run: |
-          cmake -S . -B build/san -DCMAKE_BUILD_TYPE=Debug \
+          # RTSan is clang-only — GCC rejects `-fsanitize=realtime` outright — and the attribute
+          # it keys off is [[clang::nonblocking]], which GCC ignores anyway. Passed as a CMake
+          # variable on this lane only, rather than exporting CXX: an empty CXX="" on the other
+          # lanes is not the same as unset, and some probes read it as a broken compiler path.
+          compiler=""
+          if [ "${{ matrix.kind }}" = "realtime" ]; then
+            compiler="-DCMAKE_CXX_COMPILER=clang++-20"
+          fi
+          cmake -S . -B build/san -DCMAKE_BUILD_TYPE=Debug $compiler \
             -DCMAKE_CXX_FLAGS="-fsanitize=${{ matrix.kind }} -fno-omit-frame-pointer -g" \
             -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=${{ matrix.kind }}"
           cmake --build build/san --target mm_tests -j"$(nproc)"
           # Leak detection is a different concern from the races/UAF this gate hunts, and it fires on
           # third-party static init; keep the signal on what we're actually looking for.
-          ASAN_OPTIONS=detect_leaks=0 TSAN_OPTIONS=halt_on_error=1 ./build/san/test/mm_tests
+          #
+          # RTSan does NOT halt: the render path's known blocking calls are frozen in
+          # docs/metrics/hotpath-baseline.txt and backlogged as architecture work, so halting would
+          # fail this lane on every run. It reports; the log is the signal.
+          ASAN_OPTIONS=detect_leaks=0 TSAN_OPTIONS=halt_on_error=1 \
+            RTSAN_OPTIONS=halt_on_error=0 ./build/san/test/mm_tests

CLAUDE.md

@@ -63,6 +63,8 @@ Git only with the PO in the loop: staging, committing, and pushing happen only w
 
 On "run pre-commit": `uv run moondeck/event/precommit.py`. It runs every gate whose trigger the change matches and reports PASS / FAIL / SKIP / MANUAL. Then wait for an explicit "commit now".
 
+**"commit now" applies to the diff the PO just reviewed, and any later edit cancels it.** The PO reviews every line before committing (§ Roles), so the go-ahead is scoped to the files as they stood when it was given. Change one afterwards — a review finding, a CI fix, a doc touch-up — and the order is void: say what changed and wait for a fresh "commit now". This holds however small the change and however clearly an earlier instruction seems to cover it ("we commit in one go" says how *many* commits, not *when*).
+
 Commit message: title ≤ 72 characters, imperative. Then a 1–3 sentence end-user TL;DR (no file lists). Then the performance one-liner, measured for every supported target by running `collect_kpi.py --commit` with a board attached. Then change sections as bullets: **Core**, **Light domain**, **UI**, **Scripts/MoonDeck**, **Tests**, **Docs/CI**, **Reviews** (🐇 external / 👾 Reviewer, one bullet per finding: flagged → done/accepted/deferred + why). Core and Light domain are the preferred default categories (a core-module test → Core; a script fix touching a light driver → Light domain). No hard wraps inside a part. Full performance block at the bottom.
 
 **Reviewer at commit-time:** run the Reviewer on the staged diff when the commit is large (roughly ten files or more across areas) or on PO request — start it first so the other checks run in parallel; findings fixed or accepted-with-reason before "commit now".

CMakeLists.txt

@@ -42,7 +42,28 @@ if(MSVC)
     # Static MSVC runtime so the Windows binary doesn't need vcredist.
     set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 else()
-    add_compile_options(-Wall -Wextra -Werror)
+    # Tier zero on top of -Wall -Wextra: five warnings that catch real defects the base set
+    # misses. -Wdouble-promotion is the one that earns its place here specifically — the Xtensa
+    # has no FPU, so an accidental float->double promotion is a silent softfloat call in the
+    # render path, and it enforces the integer-math rule the coding standards ask for.
+    # (GCC/Clang only; the MSVC branch above uses /W4 /WX, which has no direct equivalents.)
+    add_compile_options(-Wall -Wextra -Werror
+                        -Wshadow -Wnon-virtual-dtor -Wdouble-promotion
+                        -Wimplicit-fallthrough -Wnull-dereference)
+
+    # Hot-path discipline, enforced by the compiler. MM_NONBLOCKING marks tick/tick20ms/tick1s
+    # (platform.h); -Wfunction-effects then checks TRANSITIVELY that nothing they reach
+    # allocates or blocks, through the whole call graph. Clang 20+ only; the flag does not
+    # exist on GCC, so the ESP32 build gets no hot-path check — src/platform/esp32/ is
+    # analysed only where the desktop build reaches it (docs/testing.md § Static analysis).
+    if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 20)
+        # -Wno-error is PERMANENT, not a stepping stone: this is a report, not a gate. The
+        # findings are real and frozen in docs/metrics/hotpath-baseline.txt as architecture
+        # work; failing the build on them would block every other gate, and a new one may be
+        # legitimate (a driver that must wait on hardware). The clang-hotpath card reports
+        # what is NEW; a human judges it.
+        add_compile_options(-Wfunction-effects -Wno-error=function-effects)
+    endif()
 endif()
 
 # `uv` is the project's Python launcher (see CLAUDE.md / moondeck/MoonDeck.md).