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.tests passes locally, all 41 cases.
  • No changes to production code (parse_trade_instrument.cpp, wire_codec.cpp) – the msgpack default itself is correct.
  • systemd_notify.cpp compiles 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

PR Title
#1814 [marketdata,qt,synthetic,agile] Sprint 24 close: systemd fix, release notes, fx spot diagnostics
#1807 [qt,service] Hotfix: fix two CI-breaking regressions (msgpack default, macOS/Windows compile)

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.

Emacs 29.3 (Org mode 9.6.15)