Task: Implement Hotfix: default wire codec flip to msgpack breaks ores.qt.headless.tests
Table of Contents
This page documents a task in the Hotfix: default wire codec flip to msgpack breaks ores.qt.headless.tests story. It captures the goal, current status, acceptance, and any notes or results.
Goal
Fix instrument_parse_dispatch_tests.cpp so its fixtures encode
through the same wire codec parse_trade_instrument() decodes with,
instead of hard-coding rfl::json::write() against a process default
that is now msgpack. Also fix ores.service's systemd_notify.cpp,
found broken on macOS/Windows CI while confirming "all operative
systems" were checked, not just Linux – it unconditionally used
Linux-only POSIX socket APIs.
Status
| Field | Value |
|---|---|
| State | DONE |
| Parent story | Hotfix: default wire codec flip to msgpack breaks ores.qt.headless.tests |
| Now | Nothing. |
| Waiting on | Nothing. |
| Next | Nothing. |
| Last touched | 2026-08-02 |
Acceptance
test_ores.qt.headless.testspasses locally, all 41 cases.- No changes to production code (
parse_trade_instrument.cpp,wire_codec.cpp) – the msgpack default itself is correct. systemd_notify.cppcompiles on all platforms; Linux behaviour unchanged (same code, now inside#if defined(__linux__)).
Plan
Root cause, found by bisecting Continuous Linux run history via
gh run list=/=gh run view --log: CI has been red on every run since
e7a49f8c1a (last green, 2026-07-30 20:53 UTC) →
95a10b4e84 (first red, 2026-07-31 01:44 UTC). That specific window's
break was a separate, since-resolved issue (PR #1788's tenant_id
removal, reverted by 2e9d82d6a). The failure that is still red at
current main tip is different: ores.qt.headless.tests failing
instrument_parse_dispatch_tests.cpp's
parse_trade_instrument_dispatches_*=/=returns_nullopt_for_* cases
with result.has_value() for: false, present in every run since
66b8ccd0d (2026-08-01 05:15) through the current tip.
Traced to b34181e6a ("[nats,qt,compass] Wire ORES_NATS_WIRE_FORMAT
into the Qt client and flip the default to msgpack", merged 2026-07-31
18:13 as part of PR #1793): it flipped
wire_codec.cpp's compiled-in mutable_default_wire_codec() from
wire_format::json to wire_format::msgpack. That default is what
parse_trade_instrument.cpp's try_parse<T>() always decodes
through (ores::nats::default_wire_codec().decode<T>(bytes)), but
instrument_parse_dispatch_tests.cpp's make_response_json() helper
(and one inline call site) still encoded fixtures with
rfl::json::write() directly – JSON bytes fed to a msgpack decoder,
so every dispatch case's parse_trade_instrument() call returned
std::nullopt.
Fix: added an encode_via_default_codec() helper that encodes
through ores::nats::default_wire_codec() (returns
std::vector<std::byte>, converted to a binary-safe std::string)
and replaced both rfl::json::write() call sites with it. The
fixture now round-trips correctly regardless of which wire format is
the current process default – fixes today's msgpack default and
won't silently break again if the default changes again later.
Dropped the now-unused <rfl/json.hpp> include.
Second regression: macOS/Windows compile failure
Asked explicitly whether CI had been checked "across all operative
systems" – it had not; only Linux logs had been inspected up to that
point. Pulled the latest Continuous MacOS and Continuous Windows
run logs directly via gh run view --log, and found a second,
unrelated failure: both fail to even compile
ores.service/src/service/systemd_notify.cpp. macOS: error:
'sockaddr_un' does not refer to a value / use of undeclared
identifier 'sun_path'. Windows: fatal error: 'sys/socket.h' file
not found. Same file, same root cause on both: the file
unconditionally #include=s =<sys/socket.h>=/=<sys/un.h> and uses
sockaddr_un=/=AF_UNIX – POSIX/Linux-only APIs implementing the
sd_notify(3) wire protocol, added by the systemd sd_notify-readiness
work (merged as part of PR #1793/#1802) with no platform guard, even
though its own header doc already says the function is "safe to call
unconditionally" (true only if it compiles everywhere).
Confirmed no other file in the tree has the same unguarded-include
pattern (grep -rl "sys/socket.h\|sys/un.h" projects/ – only this
one file).
Fix: wrapped the whole Linux implementation in
#if defined(__linux__) ... #else ... #endif, with a no-op body on
the #else branch (macOS/Windows have no systemd to notify; a no-op
is exactly the documented contract already, just now actually true
cross-platform). Linux behaviour is byte-for-byte unchanged – same
code, just now conditionally compiled. Verified the Linux build still
compiles clean; macOS/Windows compilation cannot be verified locally
(no cross-toolchain in this environment) and will be confirmed by CI
on the PR.
Follow-up: replace raw POSIX sockets with boost::asio (round 2)
User pushed back on the raw socket()=/=sendto()=/=sockaddr_un
POSIX code as a hack, given the codebase already depends on
Boost.Asio everywhere else (ores.nats etc.) and Asio wraps AF_UNIX
datagram sockets directly. Rewrote using
boost::asio::local::datagram_protocol::socket=/=endpoint, guarded
by #if defined(BOOST_ASIO_HAS_LOCAL_SOCKETS) (Asio's own macro for
"local sockets are available here", true on POSIX, false on Windows)
rather than __linux__ directly – same practical effect (still a
no-op on Windows; macOS could theoretically compile the real path
too, though $NOTIFY_SOCKET is never set there in practice since
macOS has no systemd) but ties the guard to what the code actually
needs (Asio local-socket support) rather than a platform name.
Removed all manual sockaddr_un=/=offsetof=/=memcpy packing –
boost::asio::local::datagram_protocol::endpoint(path) builds the
address, and socket::send_to() replaces ::socket()=/::sendto()=/
::close(). The abstract-socket (leading @=/NUL byte) handling
and the exact wire message (=READY=1) are preserved exactly.
try/catch (...) replaces manual return-on-error checks, keeping
the function's noexcept contract (Asio throws
boost::system::system_error on failure; caught and swallowed,
consistent with the original's best-effort semantics).
Verified: full local build clean (linux-clang-debug-make), full
ctest 74/74 passed (re-run after this change, not just the earlier
targeted ores.service.lib build). No unit tests exist for
notify_systemd_ready() itself (before or after) – it is a
fire-and-forget, environment-gated call with no meaningful behaviour
to assert against outside a real systemd socket; not adding one now,
consistent with the function's existing untested status.
Verifying cross-platform: manually dispatched Continuous MacOS/Windows against this branch
Local Linux verification alone doesn't prove the macOS/Windows
compile fix actually works – no cross-toolchain available in this
environment. Both Continuous MacOS and Continuous Windows
workflows support workflow_dispatch, so triggered both directly
against this branch (gh workflow run ... --ref
feature/implement-fix-wire-codec-default-breaks-headless-instrument-parse-tests)
rather than waiting for the next main-scheduled run or merging blind.
Found a third real, distinct issue this way:
windows-clang-debug-ninja failed with
-Werror=deprecated-declarations on std::getenv (Windows/MSVC STL
flags it in favour of _dupenv_s) – confirming along the way that
BOOST_ASIO_HAS_LOCAL_SOCKETS genuinely is defined on this Windows
toolchain (modern Windows supports AF_UNIX), so the real
Asio-local-socket code path was compiling there too, just tripping on
the unrelated getenv call. Fixed by using the project's existing
ores::platform::environment::environment::get_value() helper
(already used elsewhere in the codebase, e.g.
ores.platform/src/environment/environment.cpp itself defines
_CRT_SECURE_NO_WARNINGS internally to swallow this exact warning)
instead of calling std::getenv directly – consistent with "no raw
platform APIs where a project helper already exists," same principle
as the boost::asio rewrite above. ores.platform.lib is already a
transitive PUBLIC dependency of ores.service.lib (via
ores.utility.lib), so no CMakeLists change needed.
Cancelled the in-flight Windows dispatch (doomed – all four Windows
matrix variants would hit the identical getenv warning) rather than
waiting for it to finish failing; macOS was left running since it was
not failing on this issue (macOS's libc++ doesn't deprecate
getenv), and gives an independent confirmation the Asio rewrite
compiles there too. Re-verified locally on Linux (build + link
clean) and pushed the fix; re-dispatching MacOS/Windows against the
new commit to confirm before merging.
Notes
Test Scenarios
Manual QA scenarios (scaffolded via compass add test_scenario, run
through the QA Validation Runner panel) that verify this task. Link
new ones here as they're created; the scenario doc itself links back
via its "Verifies task" field.
| Scenario | State | Notes |
|---|---|---|
PRs
Review
| Comment summary | File | Decision | Notes |
|---|---|---|---|
Result
Fixed all three issues found (msgpack-vs-JSON test fixture mismatch, non-portable systemd_notify.cpp POSIX sockets, Windows getenv deprecation warning) and merged as PR #1807. CI is green across Linux/macOS/Windows Continuous and Nightly.