From 30278ac5c90a5a222dc199d7bc0c2596fcb1482a Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 18 Jul 2026 16:30:43 -0700 Subject: [PATCH] docs: add stability and testability handoff --- STABILITY_TESTABILITY_HANDOFF.md | 372 +++++++++++++++++++++++++++++++ 1 file changed, 372 insertions(+) create mode 100644 STABILITY_TESTABILITY_HANDOFF.md diff --git a/STABILITY_TESTABILITY_HANDOFF.md b/STABILITY_TESTABILITY_HANDOFF.md new file mode 100644 index 00000000..07220aac --- /dev/null +++ b/STABILITY_TESTABILITY_HANDOFF.md @@ -0,0 +1,372 @@ +# Stability, Testability, and Upstream-Merge Handoff + +## Purpose + +This document is the plan of record for refining the fork-owned WebConfig and +MQTT observer code after the initial policy extraction and host-test work. The +goal is to improve runtime stability, long-uptime confidence, and serviceability +without broad rewrites of upstream-heavy files or creating unnecessary merge +conflicts. + +The order is deliberate: cheap CI and persistence guardrails land first, then +tests and ownership boundaries needed to make lifecycle work safe, and only +then the cooperative-shutdown refactor. Hardware soak testing validates the +result; it is not the first line of defense for the riskiest change. + +## Current Baseline + +The current branch has: + +- Native tests for MQTT presets, validation, topic templates/routing, + connection policy, packet-queue policy, payload construction, WebConfig keys, + and upstream `Utils::toHex` behavior. +- ArduinoJson pinned to 7.4.3 in the native and firmware environments. +- Representative MQTT firmware builds passing for a constrained non-PSRAM + Heltec V3 and a PSRAM-equipped T-Beam S3 Supreme. +- WebConfig request/result correlation, failed-batch reboot gating, and a + process-lifetime HTTP listener that avoids deleting an object still referenced + by an asynchronous request. +- Pure MQTT policy helpers that reduce decision duplication while leaving the + production bridge as the integration point. + +This is a useful unit-test foundation, but it does not yet validate the MQTT +task/client lifecycle, cross-core state ownership, preference-file migrations, +or long-running heap behavior. + +## Constraints + +1. MQTT preference-file layouts are fleet-critical. Unknown newer formats must + not be overwritten, and every supported old layout must remain recoverable. +2. Upstream MeshCore changes are merged regularly. Prefer additive fork-owned + helpers and small adapters over reorganizing upstream-heavy files. +3. The non-PSRAM profile is the memory-pressure baseline; the PSRAM profile is + a separate allocation/lifecycle path and must also be tested. +4. Risky lifecycle changes require fast deterministic tests before hardware + soak testing. +5. Preserve observable behavior unless a behavior change is explicitly named, + reviewed, and tested. + +## Sequenced Work + +### Phase 0: Record the pre-change lifecycle characterization + +Capture current behavior before changing shutdown mechanics. This provides a +reference for the lifecycle fakes and lets the later state-machine refactor +prove equivalence rather than relying on memory or a multi-hour soak. + +Characterize at least: + +- Normal `begin() -> connect -> end()` ordering. +- `end()` while disconnected, connecting, connected, publishing, retrying, and + applying a slot reconfiguration. +- Which callbacks can arrive during and after `end()`. +- Queue disposition and connection/status counters across stop and restart. +- Heap, largest allocatable internal block, task stack high-water mark, and + client/task counts before start, after start, after stop, and after restart. +- Partial initialization failures: queue allocation, task creation, client + allocation, and PSRAM-buffer allocation. + +Use instrumented hardware logging where necessary, but encode every behavior +that can be represented deterministically into the lifecycle fake tests in +Phase 4. Store representative logs as CI or test artifacts rather than enabling +permanent high-volume production logging. + +### Phase 1: Put MQTT/WebConfig firmware smoke builds in PR CI + +The regular PR build matrix does not currently compile an MQTT observer target. +Add two required smoke builds for changes touching firmware, variants, +PlatformIO configuration, MQTT/WebConfig helpers, or their tests: + +- `Heltec_v3_repeater_observer_mqtt` for the constrained non-PSRAM path. +- `T_Beam_S3_Supreme_SX1262_repeater_observer_mqtt` for the PSRAM path. + +Keep the native suite required. Add ASan/UBSan to a separate native job if the +PlatformIO native toolchain supports it reliably. Record static RAM and flash +usage and fail only on reviewed limits with enough headroom to avoid noisy +one-byte regressions. + +Acceptance criteria: + +- Pull requests cannot merge when native tests or either representative MQTT + build fails. +- The build log shows ArduinoJson 7.4.3 for both firmware profiles. +- CI checks that every ArduinoJson declaration remains pinned to 7.4.3. +- Build-size output is retained as an artifact or job summary. + +### Phase 2: Fix PSRAM restart resource symmetry + +PSRAM-backed raw-data and JSON buffers are allocated in the bridge constructor, +freed by `end()`, and not reallocated by a later `begin()` on the same bridge +object. After a restart, raw-data caching remains unavailable and JSON +publication uses task-stack fallback buffers. + +Extract symmetric runtime-resource operations, for example: + +- `allocateRuntimeBuffers()` called by `begin()` or a shared initialization + path. +- `releaseRuntimeBuffers()` called by `end()` and initialization rollback. + +The operations must be idempotent, handle partial allocation, and preserve the +existing graceful fallback when PSRAM allocation fails. Avoid changing client +or task shutdown behavior in this phase. + +Tests and validation: + +- Allocation success, partial failure, repeated allocation, and repeated + release using a narrow allocator fake. +- Multiple `begin()/end()` cycles do not lose the raw-data buffer or permanently + move JSON serialization onto the task stack. +- Initialization failure releases only resources owned by that attempt. +- Representative PSRAM and non-PSRAM firmware builds pass. +- A short hardware restart loop shows stable free heap and largest free block. + +### Phase 3: Add binary MQTT preference migration fixtures + +Build deterministic tests around the versioned `/mqtt_prefs` loader before +upstream merges change `CommonCLI`, filesystem behavior, or preference structs. +Use checked-in, non-secret binary fixtures or byte arrays representing each +deployed layout. + +Required cases: + +- Pre-slot layout. +- Three-slot layout. +- Legacy headerless six-slot layout. +- Shorter version-1 payload with newer fields defaulted. +- Current version round trip. +- Truncated header and truncated payload. +- Invalid magic and implausible payload length. +- Unsupported newer version: run with safe defaults and preserve the original + file without overwriting it. +- Migration failure or interrupted save leaves a recoverable source file. +- Existing credentials, slot ordering, and publish flags survive migration. + +Prefer extracting a fork-owned serializer/decoder seam over host-compiling all +of `CommonCLI`. Keep the production adapter small and retain the frozen-layout +`static_assert`s. + +Acceptance criteria: + +- All known deployed layouts have a fixture and field-by-field expected result. +- Corrupt input cannot cause an out-of-bounds read or silent overwrite. +- A future `MQTTPrefs` layout change fails tests until its migration and fixture + are added deliberately. + +### Phase 4: Establish ownership and teardown test seams + +This phase is the safety net for cooperative shutdown. It must land before the +shutdown state machine. + +#### Ownership model + +Document one owner for each mutable runtime domain: + +- MQTT task: clients, slot connection state, NTP client operations, publish + counters, and the packet drain path. +- Loop task: CLI execution, preference persistence, WebConfig batch draining, + and bridge lifecycle requests. +- Producer/radio context: packet staging before queue handoff. +- Async TCP context: request parsing and immutable response handoff only. + +Replace cross-core `volatile` handshakes with an appropriate primitive: + +- Task notifications or a command queue for one-way lifecycle/reconfigure/NTP + requests. +- Atomics only for truly independent scalar state. +- Immutable published snapshots for WebConfig, CLI diagnostics, and alerting. +- A mutex only where ownership transfer or snapshot publication cannot express + the operation cleanly. + +In particular, loop/WebConfig code must not directly inspect mutable MQTT slot +objects or client counters. The MQTT task should publish a plain-data status +snapshot. + +#### Narrow lifecycle fakes + +Introduce interfaces or callbacks for only the dependencies needed to drive the +lifecycle deterministically: + +- Clock/timer. +- Task start, stop request, acknowledgment, and timeout. +- MQTT client connect/disconnect and delayed callbacks. +- Queue depth/pop/requeue behavior. +- Runtime allocator and heap measurements. +- OTA coordinator/barrier. + +Required teardown-focused tests: + +- Stop while connecting, connected, publishing, retrying, renewing a token, + running NTP, and applying a slot change. +- Callback delivered before stop, during stop, after disconnect, and after the + stop acknowledgment. +- Duplicate stop, stop before full initialization, and restart after stop. +- Timeout/fallback behavior when the MQTT task or client does not acknowledge. +- No client, queue, buffer, or task access after its owner releases it. +- Current queue and diagnostic behavior is preserved unless a change is + explicitly approved. + +### OTA Teardown Barrier: release-critical scenario + +Treat OTA teardown as a first-class test target throughout Phases 0, 2, 4, 5, +and 7. It is not merely another restart case. + +The required invariant is: firmware erase/write must not begin until MQTT +shutdown has reached a safe acknowledgment point, and the bridge must not be +restarted while flash writing is active. + +Required scenarios: + +- OTA requested while MQTT is connecting, publishing, draining retries, or + handling a callback. +- MQTT stop succeeds and OTA begins only after the teardown barrier. +- MQTT stop times out: OTA aborts safely rather than writing under uncertain + ownership. +- OTA preflight or download aborts before flashing: the bridge restarts once and + returns to the prior configured behavior. +- Successful OTA: no bridge restart is attempted before the device reboots. +- Power loss/reset at the platform-supported OTA boundaries retains a bootable + partition; this portion requires hardware/platform validation. +- Repeated failed OTA attempts do not leak heap, duplicate WiFi callbacks, or + leave the bridge permanently stopped. + +Retain the prior teardown/heap-panic reproduction as a regression artifact if +available. OTA-related lifecycle tests are release gates for any future change +to bridge teardown, OTA sequencing, MQTT client lifetime, or task ownership. + +### Phase 5: Implement cooperative MQTT shutdown + +With Phase 0 behavior recorded and Phase 4 tests in place, replace direct task +deletion with an explicit lifecycle such as: + +`Stopped -> Starting -> Running -> StopRequested -> Stopping -> Stopped` + +The exact representation can differ, but it must provide: + +- Idempotent start and stop requests. +- A stop request delivered through the established ownership channel. +- Cessation of new connects, publishes, retries, and reconfigurations. +- Ordered client/service shutdown on the MQTT task. +- A completion acknowledgment before the loop task releases queues, buffers, or + other shared resources. +- A bounded timeout with clear diagnostics and a deliberately reviewed fallback. +- Safe restart after a completed stop. +- An OTA barrier that consumes the same completion acknowledgment. + +Do not combine this phase with queue-loop deduplication, broad bridge cleanup, +or unrelated feature changes. + +Acceptance criteria: + +- All Phase 4 lifecycle tests pass, including delayed/stale callbacks. +- Characterized behavior from Phase 0 is preserved or differences are explicitly + documented and approved. +- OTA teardown-barrier tests pass. +- Repeated hardware start/stop cycles show no downward heap or largest-block + trend and no task/client-count growth. + +### Phase 6: Expand request, queue, connection, and publication integration tests + +After lifecycle ownership is stable, broaden deterministic integration coverage: + +- WebConfig POST/result/reboot/stop behavior, including lost responses, + duplicate request IDs, concurrent clients, partial command failure, and stop + with an active handler. +- MQTT queue behavior for FreeRTOS and circular-buffer adapters: overflow, + delayed retry, requeue failure, stale flush, queue ordering, and + `millis()` rollover. +- Packet succeeds/raw fails and raw succeeds/packet fails. +- Connection backoff, stable reset, breaker probe, WiFi recovery, token renewal, + and slot reconfiguration callback ordering. +- Topic and payload contracts at the bridge boundary, so enum/cast/adaptation + mistakes are covered in addition to the pure helper tests. + +Once both queue backends are covered by the same behavioral contract, consider +extracting a shared `processQueuedPacket()` that returns an outcome while each +backend retains pop/requeue/dequeue ownership. Do not make queue deduplication a +prerequisite for the stability work. + +### Phase 7: Establish uptime, memory, and fault-injection gates + +Use hardware soak tests to validate the already-tested design, not to discover +basic lifecycle errors for the first time. + +Run at least one constrained non-PSRAM board and one PSRAM board through: + +- Stable broker operation. +- Broker unavailable, rejecting authentication, and flapping. +- WiFi loss/recovery and credential changes. +- TLS handshake failures and repeated reconnect backoff. +- Queue saturation and slow broker behavior. +- Repeated slot reconfiguration and full bridge stop/start. +- WebConfig start/save/stop cycles. +- JWT renewal and NTP failure/recovery. +- OTA success, preflight abort, download failure, teardown timeout, and resume. +- At least one test crossing the 32-bit `millis()` rollover boundary, accelerated + where hardware time injection is available. + +Record and threshold: + +- Minimum free internal heap. +- Largest free internal block. +- MQTT task stack high-water mark. +- PSRAM free/largest block where available. +- Queue and outbox high-water marks. +- Connect/disconnect/retry/publish counters. +- Watchdog and reset reason. +- Task/client counts across restart cycles. + +Recommended gates are a 72-hour fault-injection run followed by a seven-day +stable run. Store machine-readable time series and a concise summary artifact. +Avoid enabling high-volume diagnostic logging in production builds. + +## Continuous Upstream-Merge Discipline + +Apply these practices throughout every phase: + +- Keep fork-owned logic in additive helper files and keep adapters in + upstream-heavy files small. +- Avoid drive-by formatting, renames, and unrelated cleanup in + `MQTTBridge.cpp`, `CommonCLI`, and the role-specific `MyMesh` files. +- Use single-purpose commits with messages that describe their full scope. +- Merge upstream frequently enough that conflicts remain attributable. +- Before each upstream merge, record the native-test, representative-build, + firmware-size, and persistence-fixture baseline. +- After the merge, run native tests, both MQTT smoke builds, preference fixtures, + and the relevant lifecycle/OTA tests before resolving the merge as complete. +- Review semantic behavior at adapters even when Git reports no textual + conflict; upstream signature, lifetime, and task-context changes can invalidate + fork assumptions silently. +- Reuse resolutions only after revalidating them against the new upstream code. +- Extract repeater/room-server WebConfig ownership into a shared fork helper when + that integration next requires material change; do not perform a standalone + broad move solely for aesthetics. + +## Explicitly Deferred Debt + +These items are recognized but are not prerequisites for the sequenced +stability work: + +- Shared packet-drain processing between FreeRTOS and circular-buffer backends. +- Repeater/room-server WebConfig integration duplication. +- Moving the WebConfig HTML generator out of the common ESP32 build path. +- The secret-placeholder edge case for a credential exactly equal to the UI + sentinel. +- Secure setup-AP authentication beyond the currently documented open/optional + PSK threat model. + +Revisit an item when its code is already being changed, when operational data +raises its priority, or when tests make the refactor substantially safer. + +## Completion Definition + +This roadmap is complete when: + +- PR CI protects native logic and both representative firmware memory paths. +- Every deployed MQTT preference layout has a passing migration fixture. +- Runtime resources survive repeated start/stop cycles symmetrically. +- Cross-core ownership is explicit and diagnostics consume immutable snapshots. +- Cooperative shutdown passes deterministic lifecycle and OTA-barrier tests. +- Hardware fault-injection and stable soaks meet reviewed memory, stack, task, + and reliability thresholds. +- Upstream merges use the same automated gate and do not require broad rewrites + of fork-owned behavior.