fix(PhoneAPI): convert getFromRadio state recursion to iteration

[cvaldess]

Summary

PR #10413 (2.8 NodeDB shrink) introduced four new STATE_REPLAY_* states in PhoneAPI::getFromRadio that each transition into the next via return getFromRadio(buf);. When the client does not opt into gradient sync (the standard meshtastic-python CLI path — i.e. meshtastic --info, meshtastic --set …, etc.), all four replay states are no-ops but are still walked recursively, producing 5–6 nested getFromRadio() frames. Each frame locally allocates large protobuf objects (meshtastic_NodeInfo and meshtastic_MeshPacket, ~250–400 B), so on platforms with a tight task stack the function prologue of the next frame faults during register save → HardFault → watchdog reset.

This patch replaces the six recursive return getFromRadio(buf) calls with goto retry_state; to a label placed just before the switch. The state machine now iterates within a single stack frame, preserving identical observable behavior for clients but eliminating unbounded stack growth.

Reproduction

• Board: WIZnet W5500-EVB-Pico2 (RP2350) running develop with MESHTASTIC_EXCLUDE_BLUETOOTH (so CLI is the only config path).
• Steps: boot device, run meshtastic --port COM41 --info or meshtastic --host <ip> --info from any host.
• Result before fix: USB CDC drops + Windows reconnect chime; TCP socket closes; firmware reboots. Last serial line is always [ServerAPI] Done sending N of M nodeinfos millis=…. The first log inside STATE_REPLAY_POSITIONS never prints.
• uxTaskGetStackHighWaterMark() measured 292 words / 1168 bytes free at that point — the next recursive frame overflows.

The arduino-pico FreeRTOS port hardcodes the CORE0 task stack to 1024 words / 4 KB (cores/rp2040/freertos/freertos-main.cpp:149). This is the most exposed platform; targets with larger task stacks (ESP32 typical 8 KB+, nRF52 default 4 KB but with smaller protobuf ABI) are likely to survive but with much less margin than they had pre-#10413.

What changed

1. Added a retry_state: label immediately before switch (state) in getFromRadio.
2. Replaced the six recursive return getFromRadio(buf); call sites with goto retry_state;:
   • STATE_SEND_OTHER_NODEINFOS done-branch
   • STATE_REPLAY_POSITIONS empty-replay-queue branch
   • STATE_REPLAY_TELEMETRY empty-replay-queue branch
   • STATE_REPLAY_ENVIRONMENT empty-replay-queue branch
   • STATE_REPLAY_STATUS empty-replay-queue branch
   • STATE_REPLAY_POSITIONS skip-when-no-gradient-sync (added; see below)
3. Scoped the LockGuard in STATE_SEND_OTHER_NODEINFOS done-branch so it is released before the loop iterates back. Today this is a no-op given concurrency::Lock is non-recursive and the recursive frame did not actually re-acquire it on the legacy-client path, but the explicit scoping documents intent and prevents future foot-guns.
4. Added an early skip in STATE_REPLAY_POSITIONS that jumps directly to STATE_SEND_FILEMANIFEST when clientWantsGradientSync() is false. Saves three trivial loop iterations on every legacy-client connection.

No protobuf, on-disk schema, or wire-format change. Behavior for gradient-sync-aware clients is unchanged — the four replay phases still execute end-to-end, just iteratively.

Test plan

• Build clean for wiznet_5500_evb_pico2_e22p (RP2350 + W5500 Ethernet, BLE excluded).
• On-hardware: meshtastic --port COM41 --info and meshtastic --host 192.168.1.x --info both complete normally — full config dump returned, no reset, full owner / preferences / module-config / nodes printed.
• On-hardware: subsequent --set commands accepted and persisted; reboot retains config.
• Build verification across CI matrix — all 60+ targets green on 93cc59e (rp2040, rp2350, nRF52840, esp32/s3/c3/c6, stm32, native, docker matrix).
• Gradient-sync client testing welcome — I do not have a client that opts into the special nonces, so I have not exercised the actual replay paths beyond verifying they still compile and the iteration boundaries are correct by inspection.

Notes

• Bug class is recursion-unbounded-by-state-count, not a one-off oversight; if more states get added in the future, the iterative shape makes them safe by default.
• An alternative fix would be to bump the arduino-pico task stack, but that is framework-side and would only paper over the issue.
• Issue with full diagnostic detail: filed alongside as #XXXX (will update once filed).

🤖 Generated with Claude Code

[caveman99]

Can you please test with a gradient-sync client this actually still works as intended? Apart from that LGTM

[cvaldess]

@caveman99 thanks for the approve. To answer your testing ask:

I don't have a gradient-sync-capable client on hand, but the behavior change for gradient-sync clients is zero by construction:

• The new clientWantsGradientSync() early-skip at the top of STATE_REPLAY_POSITIONS is a no-op when the predicate is true — execution falls straight through into the same beginReplayPositions() / prefetchReplayPositions() / drain block that already existed.
• The four goto retry_state jumps replace return getFromRadio(buf) only in the empty-replay-queue terminal branches, which previously already transitioned to the next state and recursed for one more iteration. The iterative version makes one more loop pass through the switch and lands in the next state with identical side effects on replay*Order / replay*Index / replayQueue.
• beginReplay*() and prefetchReplay*() are unchanged, and they all already short-circuit internally when !clientWantsGradientSync(), so the new top-of-state skip is purely a perf shortcut for the legacy path.

End state in both gradient and non-gradient paths is STATE_SEND_FILEMANIFEST with the same data emitted in the same order. CI is green across the full matrix (rp2040, rp2350, nRF52840, esp32/s3/c3/c6, stm32, native, docker) on 93cc59e.

If anyone reading this has a gradient-sync client handy I'd appreciate a quick smoke test, otherwise I'm comfortable that the diff is behavior-preserving for that path.

[cvaldess]

FYI re: the failing check-label — it's a stale-payload issue, not a missing label.

Timeline:

• 2026-05-10T22:03:21Z — PR opened, pr_enforce_labels.yml workflow queued (waiting for first-contributor approval). Payload snapshot at this moment had labels: [].
• 2026-05-10T22:03:30Z — github-actions[bot] applied the bugfix label (9 s later).
• 2026-05-11T10:32:34Z — workflow finally executed, ~12 h later, but context.payload.pull_request.labels is the snapshot from the trigger event, not the current label state → setFailed.

A new run (25666868882) was triggered when I edited the PR body just now and is sitting in action_required — once a maintainer approves it, it should pass since the payload now includes bugfix. Alternatively, toggling the bugfix label off/on would emit a fresh labeled event with a correct payload.

Sorry for the noise, the root issue is in the workflow itself (pr_enforce_labels.yml reads from the trigger snapshot rather than re-querying current labels at execution time).

[cvaldess]

Closing — superseded by #10459 (thebentern), which removed the recursive STATE_REPLAY_POSITIONS/TELEMETRY/ENVIRONMENT/STATUS chain entirely and moved satellite-DB replay into STATE_SEND_PACKETS interleaved with live traffic.

The stack-overflow root cause (4 recursive getFromRadio() frames with MeshPacket on a 4 KB task stack) no longer exists in develop since the code path it was fixing is gone.

Thanks @caveman99 for the review.