support non-local remote transports by dlipicar · Pull Request #57 · logos-co/logos-cpp-sdk

dlipicar · 2026-04-24T16:52:05Z

No description provided.

Copilot

Pull request overview

Adds support for non-local remote transports by introducing a new “plain C++” TCP/TLS transport stack (Boost.Asio-based) with explicit transport configuration and wiring it into the existing Logos transport factory and API surface.

Changes:

Introduces plain wire protocol primitives (messages, framing, JSON/CBOR codecs) and a TCP/TLS RPC runtime (client/server, connection management).
Extends SDK API to support explicit per-instance / per-client transport configuration (including multi-transport publishing on the provider side).
Adds unit tests for framing and codecs, and updates build + Nix/flake dependencies to include Boost/OpenSSL/nlohmann_json.

Reviewed changes

Copilot reviewed 49 out of 50 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
tests/sdk/test_rpc_framing.cpp	Adds unit tests for frame encoding/decoding and incremental reads.
tests/sdk/test_plain_transport_tcp.cpp	Adds (currently disabled) end-to-end transport smoke test scaffold.
tests/sdk/test_json_codec.cpp	Adds round-trip unit tests for JsonCodec message shapes.
tests/sdk/test_cbor_codec.cpp	Adds round-trip unit tests for CborCodec and a size sanity check.
tests/sdk/CMakeLists.txt	Registers new SDK test sources in `sdk_tests`.
nix/include.nix	Installs plain transport headers/sources into the Nix SDK include output.
nix/default.nix	Adds Boost/OpenSSL/nlohmann_json to Nix build inputs.
flake.nix	Adds Boost/OpenSSL/nlohmann_json to dev shell inputs.
flake.lock	Updates pinned `logos-nix` revision/hash.
cpp/logos_types.cpp	Fixes QDataStream serialization to include `LogosResult::error`.
cpp/logos_transport_factory.h	Adds factory overloads for explicit `LogosTransportConfig`.
cpp/logos_transport_factory.cpp	Selects plain TCP/TLS transport based on config and adds explicit-config overloads.
cpp/logos_transport_config.h	Introduces Qt-free transport config types + process-global default storage.
cpp/logos_transport.h	Adds default virtual URL helpers (`bindUrl` / `endpointUrl`) for transports.
cpp/logos_transport.cpp	Implements default URL helpers using existing deterministic local-socket scheme.
cpp/logos_api_provider.h	Adds provider ctor overload supporting per-instance multi-transport publish.
cpp/logos_api_provider.cpp	Publishes provider object on one-or-many configured transport hosts.
cpp/logos_api_consumer.h	Adds consumer ctor overload supporting explicit transport config.
cpp/logos_api_consumer.cpp	Implements explicit-transport consumer path via factory overload.
cpp/logos_api_client.h	Adds client ctor overload supporting explicit transport config.
cpp/logos_api_client.cpp	Implements explicit-transport client construction.
cpp/logos_api.h	Adds explicit-transport LogosAPI ctor and client cache for transport-specific clients.
cpp/logos_api.cpp	Implements explicit-transport LogosAPI ctor and transport-specific client cache.
cpp/implementations/plain/wire_codec.h	Defines the `IWireCodec` interface and `CodecError`.
cpp/implementations/plain/rpc_value.h	Introduces Qt-free `RpcValue`/list/map/bytes types.
cpp/implementations/plain/rpc_message.h	Defines on-wire message structs + `MessageType` tagging.
cpp/implementations/plain/rpc_message.cpp	Implements `messageTypeOf()` mapping AnyMessage→MessageType.
cpp/implementations/plain/rpc_framing.h	Defines length-prefixed framing + incremental `FrameReader`.
cpp/implementations/plain/rpc_framing.cpp	Implements frame encoding and incremental decode buffer logic.
cpp/implementations/plain/rpc_connection.h	Implements async duplex RPC connection (calls, methods, events, tokens).
cpp/implementations/plain/incoming_call_handler.h	Defines provider-side dispatch interface for inbound messages.
cpp/implementations/plain/rpc_server.h	Defines TCP and TLS acceptor servers wrapping connections.
cpp/implementations/plain/rpc_server.cpp	Implements async accept loops and connection lifecycle management.
cpp/implementations/plain/io_context_pool.h	Adds shared Asio `io_context` + worker thread wrapper.
cpp/implementations/plain/io_context_pool.cpp	Implements shared pool lifetime and worker thread management.
cpp/implementations/plain/json_mapping.h	Declares shared RpcValue/Message ↔ nlohmann::json mapping layer.
cpp/implementations/plain/json_mapping.cpp	Implements mapping (including base64url bytes tagging).
cpp/implementations/plain/json_codec.h	Declares JSON text codec implementation.
cpp/implementations/plain/json_codec.cpp	Implements JSON dump/parse payload codec.
cpp/implementations/plain/cbor_codec.h	Declares CBOR codec implementation.
cpp/implementations/plain/cbor_codec.cpp	Implements CBOR to_cbor/from_cbor payload codec.
cpp/implementations/plain/qvariant_rpc_value.h	Declares Qt↔plain adapters and method-metadata conversions.
cpp/implementations/plain/qvariant_rpc_value.cpp	Implements QVariant/QJson conversions and LogosResult mapping.
cpp/implementations/plain/plain_logos_object.h	Declares LogosObject implementation backed by plain RPC connection.
cpp/implementations/plain/plain_logos_object.cpp	Implements call/method/event/token flows over the plain RPC connection.
cpp/implementations/plain/plain_transport_host.h	Declares provider-side transport host exposing ModuleProxy over TCP/TLS.
cpp/implementations/plain/plain_transport_host.cpp	Implements host publishing, event fanout, and inbound dispatch to Qt.
cpp/implementations/plain/plain_transport_connection.h	Declares consumer-side transport connection opening TCP/TLS and returning LogosObjects.
cpp/implementations/plain/plain_transport_connection.cpp	Implements TCP/TLS connect and PlainLogosObject provisioning.
cpp/CMakeLists.txt	Bumps C++ standard to C++17 and links Boost/OpenSSL/nlohmann_json; includes/install plain transport files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T21:03:00Z

+    if (!callback) return;
+    if (!m_conn || !m_conn->isOpen()) {
+        callback(QVariant());
+        return;


callMethodAsync() can invoke the callback synchronously when the connection is closed (callback(QVariant())), but LogosObject explicitly requires callbacks be delivered on a subsequent event-loop iteration. Please defer this callback (e.g., via a queued invocation/timer) to match the interface contract.

Copilot

Pull request overview

Copilot reviewed 54 out of 55 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-29T17:40:41Z

+PlainTransportHost::~PlainTransportHost()
+{
+    std::lock_guard<std::mutex> g(m_mu);
+    for (auto& [name, pub] : m_published) {
+        QObject::disconnect(pub.eventConn);
+    }
+    if (m_tcp) m_tcp->stop();
+    if (m_ssl) m_ssl->stop();
+}


PlainTransportHost::~PlainTransportHost() holds m_mu while calling m_tcp->stop()/m_ssl->stop(). RpcServer::{stop} stops connections, and RpcConnection::fail() calls IncomingCallHandler::onConnectionClosed(), which in this class locks m_mu again. That can deadlock during destruction. Consider releasing m_mu before stopping the servers (e.g., move m_tcp/m_ssl out under the lock, unlock, then stop), and disconnect signals without holding the same mutex across stop().

Copilot · 2026-04-29T17:40:42Z

+    if (transports.empty()) {
+        // Back-compat: one host, chosen by the global mode + transport config.
+        m_transports.push_back(LogosTransportFactory::createHost(m_registryUrl));
+    } else {
+        // One host per configured transport — lets a single provider serve
+        // its object on several endpoints simultaneously (local-socket +
+        // TCP, TCP + TCP+SSL, etc.).
+        for (const auto& cfg : transports)
+            m_transports.push_back(LogosTransportFactory::createHost(cfg, m_registryUrl));
+    }


LogosTransportFactory::createHost(...) can return nullptr (e.g., PlainTransportHost::start() failure), but LogosAPIProvider unconditionally stores the result and later dereferences it in the destructor and publishProvider(). This will crash on startup failures or misconfiguration. Filter out null hosts (and fail fast if none are created), and add null checks in the destructor/publish loop to keep the provider robust.

Copilot · 2026-04-29T17:40:42Z

 LogosAPIProvider::~LogosAPIProvider()
 {
    if (!m_registeredObjectName.isEmpty()) {
-        m_transport->unpublishObject(m_registeredObjectName);
+        for (auto& t : m_transports) t->unpublishObject(m_registeredObjectName);
    }


LogosAPIProvider::~LogosAPIProvider() dereferences every entry in m_transports without guarding against nullptr. This becomes a crash if any createHost(...) call failed and returned nullptr. Even if you filter nulls at construction time, a defensive null check here would prevent teardown crashes in edge cases.

Copilot · 2026-04-29T17:40:42Z

+        const QString capabilityToken = getToken("capability_module");
+        const QString origin = m_origin_module;
+        auto* consumer = m_consumer;
+        auto outerCallback = std::move(callback);
+        m_capability_consumer->invokeRemoteMethodAsync(
+            capabilityToken,
+            QStringLiteral("capability_module"),
+            QStringLiteral("requestModule"),
+            QVariantList() << origin << objectName,
+            [consumer, objectName, methodName, args, timeout,
+             outerCallback = std::move(outerCallback)]
+            (const QVariant& tokenResult) mutable {
+                consumer->invokeRemoteMethodAsync(
+                    tokenResult.toString(),
+                    objectName, methodName, args,
+                    std::move(outerCallback), timeout);
+            },


The requestModule async chain captures m_consumer as a raw pointer (auto* consumer = m_consumer) and later uses it in the capability_module callback. If LogosAPIClient (and thus its child consumers) is destroyed while requestModule is in flight, this can become a use-after-free. Prefer capturing a QPointer (or QPointer and re-read m_consumer) and bailing out if it’s null before invoking the second-stage call.

Copilot · 2026-04-29T17:40:43Z

+                    // Move the stream into a connection. SslStream is not
+                    // movable, so keep the shared_ptr and wrap a reference
+                    // — actually RpcConnection's Stream template needs a
+                    // concrete type it owns. Workaround: the Stream type
+                    // we use is SslStream itself; move out of shared ptr
+                    // via release-and-reconstruct. Simpler: store SslStream
+                    // directly in RpcConnection; use std::move here.


The comment above the SslConnection construction contradicts the implementation: it says SslStream is not movable, but the code immediately does std::move(*stream). This is misleading for future maintainers and makes it unclear what the actual constraint is. Please rewrite/remove the stale part of the comment so it accurately reflects the chosen approach.

Suggested change

// Move the stream into a connection. SslStream is not

// movable, so keep the shared_ptr and wrap a reference

// — actually RpcConnection's Stream template needs a

// concrete type it owns. Workaround: the Stream type

// we use is SslStream itself; move out of shared ptr

// via release-and-reconstruct. Simpler: store SslStream

// directly in RpcConnection; use std::move here.

// After a successful handshake, transfer ownership of

// the negotiated SSL stream into the connection object.

Copilot · 2026-04-29T17:40:43Z

+target_link_libraries(logos_sdk PUBLIC
+    Qt${QT_VERSION_MAJOR}::Core
+    Qt${QT_VERSION_MAJOR}::RemoteObjects
+    Boost::headers
+    OpenSSL::SSL


The plain transport uses Boost.Asio (and boost::system::error_code), which commonly requires linking Boost.System. Here the target links only Boost::headers; that can cause downstream link errors on platforms where Boost.System is not header-only, and Boost::headers may not exist when Boost is found via CMake’s FindBoost module. Consider requesting/linking Boost::system (and Boost::boost/headers) explicitly.

dlipicar added 3 commits April 24, 2026 13:44

support non-local remote transports

d143d25

fix LogosResult

c38f507

allow getting client over specific transport

ee0fbd1

dlipicar marked this pull request as ready for review April 24, 2026 20:55

Copilot AI review requested due to automatic review settings April 24, 2026 20:55

Copilot AI reviewed Apr 24, 2026

View reviewed changes

fix ssl

89b0be9

dlipicar force-pushed the support-non-local-remote-transports branch 3 times, most recently from 20487b1 to 7403584 Compare April 25, 2026 00:12

investiage ssl error

4c68f68

dlipicar force-pushed the support-non-local-remote-transports branch from 7403584 to 4c68f68 Compare April 25, 2026 00:25

dlipicar added 2 commits April 26, 2026 18:12

pr comments

da7fd66

allow transport set configuration on any module

0de683a

dlipicar force-pushed the support-non-local-remote-transports branch from d162db5 to 0de683a Compare April 28, 2026 20:17

dlipicar requested a review from Copilot April 29, 2026 17:33

Copilot started reviewing on behalf of dlipicar April 29, 2026 17:34 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

pr comments

9a7607a

Conversation

dlipicar commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants