Followup to #10 (api_client FIFO) — invariant documentation, not a known-reachable bug
#10 replaces the single-slot response handler with a positional FIFO queue: each response/error pops the front entry (table.remove(self._queue, 1)). It is correct today. This issue records the invariants that correctness depends on, so a future change doesn't silently break them — after tracing the transport, none of the failure modes are reachable in current code.
The invariant:
exactly one terminal event (response XOR error) per request, delivered in send order, on the same live transport+queue.
Why it holds today:
- No reordering. The worker (
mqtt_thread.lua) is a single thread: it pops one command, runs its HTTP request to completion (blocking), pushes exactly one http_response/http_error, then pops the next. Requests never overlap, so positional matching is correct by construction.
- Exactly one event per request.
request_with_retry is bounded and every handle_http_* does if status then response else error — even a timeout emits http_error. Zero-event only on a hard worker-thread crash.
- No in-place transport swap.
self.mqtt is assigned once in api_client.new and never reassigned. MPAPI.reconnect = disconnect() + connect(), and connect() builds a fresh api_client (lifecycle.lua:121) with a new empty _queue — the old client and its queue are discarded wholesale. So there is no "surviving client whose transport was swapped mid-flight." (The _install_router comment about "a reconnect that swaps self.mqtt" describes a scenario the current lifecycle doesn't actually produce — the re-application is harmless but not load-bearing.)
What would break it (future changes to watch)
- Reusing one
api_client across reconnects (swapping self.mqtt in place instead of rebuilding) — the stale _queue would misroute every response after. If anyone adds in-place reconnect, also flush+error the queue on swap.
- A concurrent / pipelined worker (more than one in-flight HTTP at a time) — breaks the send-order == answer-order guarantee; would need id-tagged matching (monotonic id echoed by the worker, match by id not position). Touches
mqtt_thread.lua.
- A caller that sends without enqueuing, or enqueues without sending — position stops matching send order.
Cheap guard worth considering now
A dev-build assertion: warn if a response arrives with an empty _queue. Zero cost in prod, surfaces any future desync immediately instead of as silent misrouting.
Priority
Low. Documentation + a tripwire, not a fix — there is no reachable failing case to write a regression test against today. Revisit if in-place reconnect or concurrent requests are ever introduced.
Followup to #10 (api_client FIFO) — invariant documentation, not a known-reachable bug
#10 replaces the single-slot response handler with a positional FIFO queue: each response/error pops the front entry (
table.remove(self._queue, 1)). It is correct today. This issue records the invariants that correctness depends on, so a future change doesn't silently break them — after tracing the transport, none of the failure modes are reachable in current code.The invariant:
Why it holds today:
mqtt_thread.lua) is a single thread: it pops one command, runs its HTTP request to completion (blocking), pushes exactly onehttp_response/http_error, then pops the next. Requests never overlap, so positional matching is correct by construction.request_with_retryis bounded and everyhandle_http_*doesif status then response else error— even a timeout emitshttp_error. Zero-event only on a hard worker-thread crash.self.mqttis assigned once inapi_client.newand never reassigned.MPAPI.reconnect=disconnect()+connect(), andconnect()builds a freshapi_client(lifecycle.lua:121) with a new empty_queue— the old client and its queue are discarded wholesale. So there is no "surviving client whose transport was swapped mid-flight." (The_install_routercomment about "a reconnect that swaps self.mqtt" describes a scenario the current lifecycle doesn't actually produce — the re-application is harmless but not load-bearing.)What would break it (future changes to watch)
api_clientacross reconnects (swappingself.mqttin place instead of rebuilding) — the stale_queuewould misroute every response after. If anyone adds in-place reconnect, also flush+error the queue on swap.mqtt_thread.lua.Cheap guard worth considering now
A dev-build assertion: warn if a response arrives with an empty
_queue. Zero cost in prod, surfaces any future desync immediately instead of as silent misrouting.Priority
Low. Documentation + a tripwire, not a fix — there is no reachable failing case to write a regression test against today. Revisit if in-place reconnect or concurrent requests are ever introduced.