mirror of
https://github.com/logos-co/logos-protocol.git
synced 2026-08-30 21:41:10 +00:00
3da8de93dfd1bc835c10ad48088d56bd6c607b97
21
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dario LipicarandClaude Opus 5
|
3da8de93df |
fix(codec): a whole-valued float still decodes as an integer (#32)
The signedness/range check in #31 went one step too far: it rejected 3.0 for an `int`, not just 3.7. That broke four long-standing test_basic_module_cpp cases (`addInts(3.0, 4.0)`, `echoInt(42.0)`, `isPositive(5.0)`, `twoArgs(hi, 3.0)`) which pass a whole-valued double where the contract declares an integer. They are right and the check was wrong. JSON does not distinguish 3 from 3.0, and this codec already says so in the other direction — Codec<double> accepts an integral number because "2 and 2.0 are the same value to JSON, and every encoder that sees a whole double may emit either". The two directions have to agree. It also matters in practice rather than in principle: logoscore's CLI types its arguments by parsing, so `logoscore call m addInts 3.0 4.0` produces JSON floats. Refusing them rejects a caller over a spelling of the same number. So a float decodes as an integer when it has no fractional part and fits; 3.7 is still refused, which is what the original change was actually for. Bounds are strict on the upper end for the same reason as the QJsonValue guard: double(int64max) rounds UP to 2^63, so `<=` would admit a value the cast cannot represent. verified: test-modules 176/176 with the four cases green again, and the conformance matrix unchanged at 170 pass / 2 xfail — hostile/int/fractional still expects dispatch_failed and gets it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 5
|
c0df466172 |
fix: integer signedness in the codec, and a shape check on the pending-call sentinel (#31)
* fix(codec): signedness and range are part of the integer type Codec<T>::from accepted any integral JSON number and handed it to .get<T>(). That is silent in both directions: .get<uint64_t>() on -1 -> 18446744073709551615 (a sign flip) .get<int32_t>() on 2^40 -> truncated Both now reject with the usual path-carrying CodecError instead. Rejecting is the codec's existing contract — a value the declared type cannot represent must not reach business logic wearing a different one — this just extends it to the half of the integer domain it was skipping. Note the check is on the JSON category, not the value: a negative literal parses as number_integer and never as number_unsigned, so `is_number_unsigned()` is the reliable discriminator rather than a comparison after conversion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(async): the pending-call sentinel is matched by shape, not by key presence All four detection sites tested `m.contains(pendingCallKey())` and nothing else, so ANY user map carrying that key was taken for a deferred call: the consumer extracted a call id, found no completion, and waited out a nested event loop. The measured outcome is a ~20s HANG, not a fast failure. An `any` slot is enough to reach it — anything a user can put in a map. logos::isPendingCallSentinel now requires the canonical shape: exactly one entry, under the sentinel key, holding a non-empty string. Shape and signature are mirrored from isUnauthorizedSentinel (logos_rpc_status.h), QJsonObject arm included — the two are the same kind of in-band marker and there was no reason for them to be guarded differently. That guard, and isTaggedBytes's, both already existed in this repo; the difference was chronology, not principle. Behaviour-preserving: the generated glue builds this map with exactly one entry whose value is a QString call id, so no real sender changes. The concurrent dispatch tests pass unchanged. NARROWS, DOES NOT CLOSE — and the tests say so out loud. A one-key, string-valued forgery IS the sentinel; no predicate can separate them. It still hangs, and because call ids are a per-object counter from 0, a forged "lc-0" can collide with a genuine in-flight completion and steal its result. Closing that needs an out-of-band channel for "deferred", which the single-QVariant dispatch slot cannot express without an ABI break — the constraint is stated at logos_rpc_status.h:24-27 and is real. tests: 10 new, including one asserting the forgery still matches, so a future reader cannot mistake the green cells for "the sentinel is safe". 236/236. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 5
|
8b8a358c8b |
fix: uint64 survives the event path and the plain wire (#30)
* fix(events): the event bridge converts through the canonical helper
setEventListenerStdBridge adapts the universal event callback (name + JSON
string) to the Qt EventCallback (name + QVariantList). It is the event-path
counterpart of callMethodStdBridge, but it did the conversion itself:
callMethodStdBridge -> logos::nlohmannToQVariant (canonical)
setEventListenerStdBridge -> QJsonDocument::fromJson
+ QJsonValue::toVariant (Qt's parser)
Two consequences, both measured by the LIDL conformance matrix as M6:
* a uint64 above int64max degraded to a double. Qt 6 backs QJsonValue with
QCborValue, so integers up to int64 DID survive — only values with no
integral representation there fell back to double. echoUint(2^64-1) was
exact while uintEvent(2^64-1) arrived as 1.8446744073709552e+19: same
value, same process, one hop later.
* canonical tagged bytes {"_bytes": ...} were not decoded, arriving as a
QVariantMap where the method path yields a QByteArray. This never showed up
end-to-end because the undecoded map round-trips to JSON and the python
client decodes the tag itself — but a C++ or QML event subscriber got a map.
Both now go through logos::nlohmannArgsToQVariantList, which the generated
cdylib emitTrampoline already used. Numbers and bytes no longer depend on
whether a value left the module as a return or as an event.
Not the residue of the codec convergence, despite how M6 was originally
registered. #29 converged six copies of the VALUE codec; this was a seventh
conversion inside an ADAPTER, which that scope never touched. It is also not on
the providers' own path — a Qt provider stores its callback verbatim and a
cdylib provider already converted correctly. The one live caller is the
logoscore daemon's CoreServiceImpl, which forwards every watched module event;
that is why C++ and Rust providers measured identically.
Why it survived: the bridge appeared in the test suite once, in
test_universal_provider_dispatch.cpp, purely to satisfy the pure virtual. No
test asserted anything about an event payload. The method path got 15 contract
tests in #29; the event path got none.
tests: 11 new cells pin the bridge directly — uint64 past int64max, 2^53+1,
int64::min, large integers nested in containers, tagged bytes at top level and
at depth, plus the shapes that already worked (multi-param order, double staying
double, null elements, empty payload, the non-array raw-string fallback) so a
future rewrite cannot quietly drop them. 210/210.
verified: logos-cpp-sdk, logos-qt-sdk, logos-liblogos and logos-logoscore-cli
all green against this build; the conformance matrix goes 156 -> 158 pass with
M6's two cells retired, and the ext table stays 40/40.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(events): pin the signedness rule the convergence brings with it
nlohmannArgsToQVariantList classifies every non-negative integer as unsigned, so
a LIDL `int` event argument now arrives as ULongLong where it used to be
LongLong. That matches what nlohmannToQVariant (the method path) and the cdylib
emitTrampoline already did — the surfaces now agree — but it is an observable
metatype change that nothing asserted.
Pinned in both directions (non-negative -> ULongLong, negative -> LongLong) so
it stays a decision rather than a side effect. Value-level reads are unaffected.
212/212.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(plain): RpcValue can represent a uint64 above int64max
The plain (tcp/tcp_ssl) wire squeezed every unsigned value through int64_t, so a
LIDL `uint` above int64max wrapped — independently in each direction:
outbound qvariant_rpc_value.cpp QMetaType::ULongLong -> int64_t(...)
inbound json_mapping.cpp is_number_unsigned -> get<int64_t>()
Neither wraps loudly: .get<int64_t>() past int64max returns -1 with no
exception. Two peers both running this code agreed on -1, so nothing looked
broken from inside — and no plain-tier test used an integer outside int32 range.
Measured over real tcp before the fix:
echoUint(2^63) -> -9223372036854775808
echoUint(2^64-1) -> -1
This was never a wire-format constraint. Both codecs carry uint64 natively (CBOR
emits major type 0, `1b ff..ff`) and the envelope's own `id` field already
crossed this wire as uint64_t. Only RpcValue *payloads* could not represent it.
RpcValue gains a uint64_t alternative, used through `makeInteger()` and ONLY for
values above int64max — the sole case where int64_t loses information. Anything
broader would change the representation of every non-negative integer already on
this wire, and since std::variant equality compares the alternative index it
would break comparisons against int64-built values, to fix nothing. Small
unsigned values keep crossing as signed, pinned by a test so the rule stays
visible.
Also fixes an off-by-one in the QJsonValue::Double -> int64 guard while here:
double(int64max) rounds UP to exactly 2^63, so `d <= double(int64max)` admitted
2^63 and then ran int64_t(d) out of range — undefined behaviour, saturating on
arm64 and INT64_MIN on x86-64. Now a strict `<` against 2^63.
tests: 14 new. Both codecs round-trip 2^64-1 flat and nested; negatives stay
signed; the Qt boundary is exact in both directions; the narrow representation
rule and the 2^63 guard are pinned. 226/226.
verified end-to-end, cross-process, with a negative control: the new 64-bit
boundary cases in logos-logoscore-py fail on the pinned protocol over tcp with
exactly the values above, and all 68 pass with this build — on local, tcp and
tcp_ssl alike.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 5
|
362b03fb1e |
feat(codec): one canonical LIDL ↔ JSON codec, generic over composition (#29)
* feat(codec): one canonical LIDL <-> JSON codec, generic over composition
The tagged-bytes encoding {"_bytes": "<base64url, unpadded>"} was implemented
SIX times — the Qt conversion here, the plain wire's json_mapping, the lp helper
in logos-cpp-sdk, a copy emitted into every generated cdylib module, the Rust
SDK and the Python client — and they disagreed on which inputs they accept:
- {"_bytes":"AA","x":1} decoded as BYTES on the lp path (no size()==1 check)
but as a MAP on the plain wire and in the glue.
- Padded "AH-A_w==" gave correct bytes in one copy, empty in another, None in
Rust.
- A plain string / number / number-array argument was accepted by C++
providers (Qt and CLI parity) and rejected by Rust ones.
logos_codec.h is the single implementation. Leaves: tstr, bstr, every signed and
unsigned integral spelling, every floating spelling, bool, any (recursion stops).
Composition is GENERIC — std::vector<T> and std::map/unordered_map<std::string,T>
for any supported T, at any depth — so [bstr], [[bstr]], {tstr: [bstr]} and bytes
nested in a map all encode canonically without anything enumerating combinations.
Codec<T> is a trait, so an unsupported T is an incomplete type: a compile error
naming the type, never a silent fallback. Decode throws CodecError carrying the
path ("[0][1]", ".k") instead of substituting a default — a mangled value must
not reach business logic. bstr keeps a documented lenient form for provider-side
arguments, because the Qt consumer path and the logoscore CLI both produce plain
strings and number arrays for byte parameters.
JsonArg exists for generated dispatch: it converts itself into whatever the
callee's parameter type is. Naming the type instead is a trap — spelling [uint]
as std::vector<uint64_t> (the LIDL mapping) does not bind to an author's
std::vector<uint32_t>, since distinct vector instantiations do not convert.
logos_codec.h joins the installed header set; nix/include.nix already globs
cpp/*.h.
Tests: 198/198. 15 new ones pin the contract rather than the happy path —
[[bstr]] tagged at depth, map-of-bytes, empty elements surviving as elements,
uint64 past 2^63, an integral JSON number decoding as float64, padded base64,
the multi-key {"_bytes":...} case being a map, and path-carrying failures.
Not yet converged onto this header (follow-ups): the Qt conversion in
logos_json_convert.cpp, and the plain wire's copy in json_mapping.cpp — the
latter needs a strict variant first, because it THROWS on malformed base64
(via its own logos::plain::CodecError) where every other copy is tolerant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(codec): fold the Qt and plain-wire copies into the shared codec
The two remaining in-repo implementations now delegate:
- logos_json_convert.cpp (the Qt CONSUMER path — argument encoding and return
decoding) dropped Qt's toBase64/fromBase64 and its own tagged-bytes
predicate. Only the QByteArray <-> std::vector<uint8_t> hop stays local, so
the Qt path cannot drift from the wire or from providers: same alphabet, same
padding rule, same single-key shape.
- implementations/plain/json_mapping.cpp dropped its anonymous-namespace
b64url_encode/decode.
The wire needed something the tolerant decode does not give it: it REJECTS a
corrupt frame rather than silently decoding fewer bytes. Hence
b64UrlDecodeChecked — strict about the alphabet and the length, tolerant of '='
padding — which json_mapping uses to keep throwing its own
logos::plain::CodecError. Consumer-facing decodes stay tolerant. Both behaviours
now come from one implementation instead of four that disagreed.
Also removed the local isTaggedBytes wrapper, which shadowed the shared one and
made unqualified calls ambiguous.
Tests: 199/199, with the strict decode's accept/reject set pinned (padding
tolerated, stray character rejected, impossible length rejected).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 5
|
ae2f7e1b58 |
fix(lp): create Qt-affine clients on the Qt main thread (#28)
lp_client_create() made the CALLING thread the client's owner thread. Callers reach it through a lazily-created wrapper (the generated bind_<iface>() -> LpClient::ensure()), so the first thread to make an outbound call captured the whole transport for the life of the process. For the qt_remote transport that thread also ends up owning the QRemoteObjectNode and its QLocalSocket, which are only serviced by a thread running a Qt event loop. A module whose first call came from a worker — an HTTP handler, a timer thread — bound its transport to a thread that only pumps events while it is already blocked inside a call. Replica acquisition then never completed: every requestObject() burned its full 20s timeout and returned nullptr, and since a failed acquire yields an empty result the data loss was silent. openmetrics-module hit exactly this: one GET /metrics took 40s (2 x 20s) and came back missing a module, /health went unanswered behind the wedged libmicrohttpd thread, and the follow-up stop RPC failed. Construct the client on the Qt main thread when the transport needs a Qt event loop, so the per-call marshal that already exists (logos::runOnOwnerThread) lands somewhere that can actually service it. This is the anchor the Qt path always had — LogosAPI::getClient marshals construction to the LogosAPI's thread — given to the lp path. Plain (tcp/tcp_ssl) and mock transports are Qt-free and thread-agnostic, so they keep the calling thread: a worker-thread consumer stays off the main thread's back. LogosTransportFactory::needsQtEventLoop() carries that rule next to the createConnection resolution it mirrors. When there is nothing to anchor to (a Qt-affine transport with no QCoreApplication) we now warn instead of letting it surface as a mute timeout. Tests: a worker thread creates an lp client over qt_remote and calls a provider published on the main thread; passes in ~0.15s, and with the construction hop reverted fails after 24.8s / 49.9s — the acquire timeouts themselves. Plus a truth table for needsQtEventLoop. 183/183 protocol tests pass. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 4.8
|
8ede8ece08 |
fix(lp): destroy clients on their owner thread (#27)
lp_client_destroy() called `delete` on the LogosAPIClient directly, on
whatever thread happened to release the last handle share. That thread is
not always the owner: any binding that parks a client share in a worker —
a Rust EventSubscription moved into a bridge thread, for one — runs the
destroy there when the worker exits.
Deleting the client there destroys its consumers' transport objects off
their owner thread. With Qt Remote Objects that tears down the node's
QLocalSocket and its socket notifiers cross-thread; Qt warns ("socket
notifiers cannot be enabled or disabled from another thread"), the fd
closes under the owner's event dispatcher ("Invalid socket N with type
Read, disabling..."), and the process takes SIGSEGV. Observed as
chat_module crashing on shutdown, when joining its bridge worker dropped
the last delivery_module share on that worker.
Defer the teardown to the owner thread via deleteLater() when the caller
is elsewhere, mirroring the marshaling every call path already does with
logos::runOnOwnerThread. A blocking marshal is not usable here: the owner
is typically the dispatch thread and may be blocked joining the very
worker running the destroy. Deferring is invisible to callers because the
callback guard, not the delete, enforces the ABI's "no callbacks after
this returns" contract.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 4.8
|
6401e30ae1 |
feat: group-shareable local sockets, stale-socket reaper, bind-failure detection (#20)
* feat: group-shareable local sockets, stale-socket reaper, bind-failure detection
The QtRO local transport binds each module's unix socket at 0777 & ~umask
(0755) with no way for a second OS user to reach it, discards the listen
result so a failed bind surfaces only as clients hanging, and never cleans up
the socket file — a hard-killed logos_host leaks it forever.
Add a Qt-free helper (logos_socket_paths.{h,cpp}) usable from both the qt_remote
and plain transport paths:
- applySocketPerms(path): chgrp + chmod a bound socket per LOGOS_SOCKET_GROUP /
LOGOS_SOCKET_MODE (chgrp-then-chmod so a half-applied policy is only ever
too strict). No-op when unset, so default behaviour is unchanged. Connecting
to an AF_UNIX socket needs write permission, so 0660 is what lets a group
member in.
- isSocketDead(path): S_ISSOCK && owned-by-us && non-blocking connect returns
ECONNREFUSED/ENOENT. Fails closed on any other outcome, so it never reports
a live socket or a regular file dead.
- reapStaleSockets(dir, prefix): unlink only the dead sockets, never a regular
file that shares the prefix (e.g. a *.lgx build artefact).
Wire it into RemoteTransportHost::publishObject and QtRemoteRegistry:
- construct QRemoteObjectRegistryHost empty and listen via setRegistryUrl() so
a bind failure is observed and logged (with lastError() + the socket path)
instead of leaving a silently-broken host;
- apply the socket-access policy to the freshly-bound local: socket.
The env-driven policy means every process in a node's tree (daemon, logos_host
subprocesses, their children) applies the same rule to every socket it binds
without threading config through each layer — the daemon exports the vars once.
Adds test_socket_paths.cpp (8 gtests): mode/group application, no-op default,
bad-mode rejection, live/dead/regular-file classification, and the reaper
keeping live sockets and regular files while removing only dead ones.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review: harden socket helpers (gid overflow, socket-owner check, empty-prefix guard, dedup path)
Addressing automated review feedback on the socket helpers:
- resolveGid(): validate strtoul() errno/range so an out-of-range numeric
LOGOS_SOCKET_GROUP is rejected instead of silently truncating to a wrong gid.
- applySocketPerms(): when a policy is requested, stat the path first and refuse
unless it's a socket we own (S_ISSOCK + st_uid == geteuid()), so a malformed
URL can never chmod/chown a stray file. No-op fast path when the env is unset.
- reapStaleSockets(): refuse an empty prefix (would make every dead socket the
process owns a deletion candidate).
- Extract the duplicated `localSocketFilePath()` (Qt QLocalServer name->path
rule) into a shared qt_remote/qt_socket_path.h so RemoteTransportHost and
QtRemoteRegistry can't drift.
Adds tests: non-socket path refused (mode unchanged), empty-prefix reaper no-op.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat: transport-aware token validator hook on ModuleProxy (#22)
* feat: transport-aware token validator hook on ModuleProxy
Adds an injectable authorizer so a host (the logoscore daemon) can accept tokens
the built-in issued-token scan doesn't know — specifically operator-issued named
tokens validated against a persistent store — with per-token expiry and
local_only enforced against the transport the call arrived on.
- ModuleProxy::setTokenValidator(std::function<bool(token, transportProtocol)>).
isAuthorized() consults it ONLY after the existing m_tokens + TokenManager
scan fails, so installing a validator is purely additive: it can grant, never
revoke, access the built-in path already allows. Empty (default) = today's
behaviour exactly.
- callRemoteMethod() gains a defaulted `transportProtocol` ("local"). The QtRO
local path (RemoteTransportHost) uses the default; PlainTransportHost::onCall
passes the real wire ("tcp" | "tcp_ssl", fail-closed to non-local on an
unexpected protocol) so a local_only token presented over the network is
rejected. One ModuleProxy is shared across a provider's transports, so the
transport can't be inferred — it must be threaded per call, which the defaulted
arg does without changing the QtRO replica's 3-arg call.
The daemon backs the validator with TokenStore::lookupByToken; other modules
keep the default (no validator) and are unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* review: split callRemoteMethod into explicit 3-arg + 4-arg overloads; include <utility>
Addressing review feedback:
- Replace the defaulted transportProtocol argument with two explicit Q_INVOKABLE
overloads. The Qt meta-object system matches methods by their full parameter
list and doesn't apply C++ default arguments, so the QtRO/local 3-arg call
must remain a real 3-arg method rather than relying on moc's reduced-arity
generation. The 3-arg form forwards to the transport-aware 4-arg form with
"local"; PlainTransportHost keeps calling the 4-arg form with the real wire.
- Include <utility> explicitly in module_proxy.h for std::move rather than
relying on an indirect include.
Full protocol suite green (160/160).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 4.8
|
ef24bd70d9 |
fix(protocol): re-exchange token on provider rejection (#26)
When a provider rejects a call for a stale/unrecognized token it now returns a structured "unauthorized" sentinel (logos_rpc_status.h) instead of a bare QVariant(). LogosAPIClient detects it below the typed wrapper, drops the cached token, re-runs capability_module.requestModule and retries the call once — closing the gap where a stale token was reused forever (the consumer-latched-dead failure mode) and lazily recovering the common provider-reload case. The return VALUE is the only provider->consumer channel available on every transport (qt_local/qt_remote/plain) without an ABI break, since the QtRO dispatch slot returns a single QVariant — hence a value sentinel. Backward compatible: - OLD consumers convert the sentinel identically to QVariant() for every scalar/string/LogosResult return, so they keep seeing today's empty/failed result. - OLD providers return bare QVariant(); a NEW consumer never matches the sentinel and so never re-exchanges against them. The retry is bounded to one attempt and fires ONLY on the explicit sentinel (never a legitimately-empty result), so no loops and no misfire. Downstream note: logos-qt-sdk's test_auth_token_enforcement.cpp asserts !isValid() on unauthorized calls; those become isUnauthorizedSentinel() when it re-pins (the security property — no provider dispatch — is unchanged). Tests: tests/protocol/test_token_reexchange.cpp covers provider-side emission, sync/async re-exchange+retry, bounded retry (no loop), the false-positive guard (a legit empty return must not re-exchange), and old-consumer decode. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Dario Lipicar
|
664b43f18a |
perf(qt_remote): cache the remote-object handle per name in LogosAPIConsumer (#24)
Acquiring a QtRO replica per call (acquireDynamic + waitForSource) is expensive:
under a tight loop — e.g. a proxy forwarding every method to its target, or a UI
backend driving a whole surface — it dominates and can even starve the nested
synchronous calls. Cache the LogosObject handle per object name in m_objectCache
and reuse it across calls (both the sync invokeRemoteMethod and the async
invokeRemoteMethodAsync paths); no per-call release(). A stale handle (source
went away — module unloaded / transport dropped) is detected via a new
LogosObject::isValid() (QtRO replica state == Valid) and transparently
re-acquired. The cache is released in clearObjectCache() from the destructor and
before reconnect().
- logos_object.h: add virtual bool isValid() (default true).
- qt_remote/remote_transport.{h,cpp}: RemoteLogosObject::isValid() (replica
Valid state) + a process-wide acquireCount() test hook.
- logos_api_consumer.{h,cpp}: m_objectCache + acquireCachedObject()/
clearObjectCache(); sync + async reuse the cached handle; async keeps the
QPointer guard and never releases the shared handle from its callback.
Test: RemoteEventTest.ConsumerReusesCachedHandleAcrossSyncAndAsyncCalls publishes
a provider over the qt_remote host, does 12 sync + 12 async echo calls, and
asserts every result is correct AND acquireCount() == 1 (one replica for all 24
calls). 164/164 green.
|
||
|
Dario LipicarandClaude Opus 4.8
|
d5ba950313 |
fix(json): keep nested bytes/ints tagged in qvariantToNlohmann containers (#23)
qvariantToNlohmann ran its canConvert<QJsonObject>/<QJsonArray> fallbacks BEFORE
the type-preserving QVariantList/QVariantMap recursion. A QVariantList/QVariantMap
also reports canConvert<QJson*>()==true, so a container was routed through QJson —
which has no byte type and degrades numerics to double. A nested QByteArray was
therefore flattened to a plain string, losing the canonical {"_bytes":...} tag.
Concretely this broke bstr method ARGUMENTS to cdylib (Rust) modules:
LogosProviderObject::callMethodStdBridge feeds each call arg through
qvariantToNlohmann, and a bstr arg arrives (over QtRO) as a QByteArray nested in
the QVariantList of call args. It was flattened to "hello", so the cdylib's
{"_bytes":...} decoder produced an empty Vec (e.g. echoBytes returned null). The
QtRO C++ path was unaffected (native QByteArray marshaling) and the plain-lp path
was already correct; only the container-through-QVariant leg dropped the tag.
Fix: move the container recursion (QStringList/QVariantList/QVariantMap) ahead of
the QJson fallbacks so nested elements recurse element-by-element (bytes stay
tagged, integers stay integers); only genuine QJson-typed variants reach the
fallbacks. Adds nested-bytes-in-list/map + bridge-shape regression tests.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Khushboo-dev-cpp
|
1e96004711 |
Merge pull request #19 from logos-co/fix/asyncCallErrorChannel
Fix/async call error channel |
||
|
Dario LipicarandClaude Opus 4.8
|
4775e635ff |
fix(json-convert): preserve integer types inside QVariant containers (#21)
* fix(json-convert): preserve integer types inside containers qvariantToNlohmann() kept integer QVariant types only for a top-level scalar; a QVariantList/QVariantMap fell through to QJsonValue::fromVariant, which degrades every numeric to double at every depth. So a `[int]`/`[uint]`/ `[float64]`/`[bool]` method arg (a QVariantList of ints) arrived as a float array, and the generated cdylib dispatch's strict .get<std::vector<int64_t>>() threw -> the param decoded as an EMPTY vector. Surfaced by a UI plugin driving [int] method args over QtRO. Recurse into QVariantList/QStringList/QVariantMap element-by-element so nested integers keep their type (and bytes/maps/lists keep their shape); also route the LogosResult value through the same recursion. Adds JsonConvertInts tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: pin LongLong container test with values > 2^53 Copilot review: (10, 20) survive an accidental IEEE-754 double detour, so they did not actually pin the integer-preservation regression. Use 2^53+1 and INT64_MAX, which lose precision / serialize in scientific notation if degraded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Khushboo Mehta
|
ee1c5192da | feat: plumb CallError through invokeRemoteMethodAsync | ||
|
Dario LipicarandClaude Opus 4.8
|
d7ad26d369 |
feat: ship liblogos_protocol shared library exporting lp_* (#4)
Add a `logos_protocol_shared` target that builds the same sources as a
shared library (liblogos_protocol.{so,dylib}), exporting the
language-neutral lp_* C ABI for out-of-plugin callers that bind it at
runtime via dlopen/FFI (logos-js-sdk's koffi.load, logos-rust-sdk's
callerBuildSupport) — the role liblogos_module_client previously filled.
The static `logos_protocol` archive and its EXPORT set are untouched, so
in-plugin code and find_package(logos-protocol) are unaffected; the shared
target is deliberately not exported into the CMake package.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Dario LipicarandClaude Opus 4.8
|
976bc7a9a0 |
fix: don't log call arguments in plaintext in ModuleProxy (#10)
ModuleProxy::callRemoteMethod is the choke point every cross-module call passes through on the callee side. It logged the full QVariantList of arguments at debug level, which dumped secrets — mnemonics, passwords, auth tokens, key material — into module logs in plaintext. Log only the argument count, matching the other transport call sites (LogosAPIClient/LogosAPIConsumer/LocalLogosObject/RemoteLogosObject all already log args.size() only). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Khushboo Mehta
|
84be236552 |
fix(logos_api_client): cache minted capability token on the client side
Without a client-side cache, every sync invokeRemoteMethod re-mints a fresh capability token. On Linux, QtRO's waitForFinished() spins a nested QEventLoop that dispatches queued slots mid-wait, so back-to-back calls reenter the function and each fires its own requestModule. The target stores ONE token per caller (TokenManager::saveToken replaces) — last inform wins and earlier in-flight calls arrive with a superseded token, rejected by ModuleProxy::isAuthorized as "auth token not recognized". Fix: save the minted token into the client's TokenManager after a successful requestModule on both the sync path and the async drain callback. Subsequent calls short-circuit the handshake — one mint per (client, target), no rotation. |
||
|
Dario LipicarandClaude Opus 4.8
|
b0c6f75498 |
fix(qt_remote): defer async completion delivery off the QtRO read stack (#7)
A `concurrency:"multi"` call's result comes back as a deferred completion event (`__logos_call_complete__`), delivered by RemoteEventHelper::onEventResponse — a slot fired by the replica's eventResponse signal. Cross-process, that slot runs on QtRO's read stack (QRemoteObjectNodePrivate::onClientRead). Until now the async user callback was invoked *inline* there, and that callback routinely (a) emits a module event — which the host-side ModuleProxy serializes onto the QtRO source — and (b) release()s the client object. Doing either while onClientRead is still unwinding re-enters QtRO and corrupts the node: a SIGSEGV in onClientRead (EXC_BAD_ACCESS, KERN_INVALID_ADDRESS at 0x80). This is the crash the EVM wallet backend hit from refresh_balances, which fans balance reads out to eth_rpc via call_async and then emits `balances_updated` from the gather completion. Primary fix (remote_transport.cpp): deliver the async completion callback on the next event-loop turn via QTimer::singleShot(0, m_helper, …) instead of inline, so all user code (event emits, release(), further calls) runs after onClientRead has fully unwound. m_helper is the context so the callback is dropped if the object is torn down first. Defense-in-depth for the same re-entrancy class: - remote_transport.cpp release()/disconnectEvents()/dtor: deleteLater() the helper (signal receiver) and replica (signal sender) and disconnect first, instead of deleting them inline — deleting a QObject mid-emission corrupts the connection list Qt is iterating. - module_proxy.cpp: always queue the source eventResponse emit to the owning thread (Qt::QueuedConnection), never emit inline, so a module that emits from inside a same-thread dispatch can't re-enter QtRO's source serialization. Tests (tests/protocol/test_remote_transport_events.cpp, newly wired): qt_remote LocalSocket event delivery (direct + full provider chain) and a reentrant-release regression that drives release() from inside a deferred-completion callback. The hard crash only reproduces cross-process (in-process QtRO posts the event, so the read stack has already unwound) — the cross-process guard is the wallet Anvil integration doctest, where this fix is A/B-proven: the published backend crashes on refresh_balances, the patched backend returns balances cleanly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 4.8
|
315a3a2e0a |
fix(qt_remote): defer async completion delivery off the QtRO read stack (#7)
A `concurrency:"multi"` call's result comes back as a deferred completion event (`__logos_call_complete__`), delivered by RemoteEventHelper::onEventResponse — a slot fired by the replica's eventResponse signal. Cross-process, that slot runs on QtRO's read stack (QRemoteObjectNodePrivate::onClientRead). Until now the async user callback was invoked *inline* there, and that callback routinely (a) emits a module event — which the host-side ModuleProxy serializes onto the QtRO source — and (b) release()s the client object. Doing either while onClientRead is still unwinding re-enters QtRO and corrupts the node: a SIGSEGV in onClientRead (EXC_BAD_ACCESS, KERN_INVALID_ADDRESS at 0x80). This is the crash the EVM wallet backend hit from refresh_balances, which fans balance reads out to eth_rpc via call_async and then emits `balances_updated` from the gather completion. Primary fix (remote_transport.cpp): deliver the async completion callback on the next event-loop turn via QTimer::singleShot(0, m_helper, …) instead of inline, so all user code (event emits, release(), further calls) runs after onClientRead has fully unwound. m_helper is the context so the callback is dropped if the object is torn down first. Defense-in-depth for the same re-entrancy class: - remote_transport.cpp release()/disconnectEvents()/dtor: deleteLater() the helper (signal receiver) and replica (signal sender) and disconnect first, instead of deleting them inline — deleting a QObject mid-emission corrupts the connection list Qt is iterating. - module_proxy.cpp: always queue the source eventResponse emit to the owning thread (Qt::QueuedConnection), never emit inline, so a module that emits from inside a same-thread dispatch can't re-enter QtRO's source serialization. Tests (tests/protocol/test_remote_transport_events.cpp, newly wired): qt_remote LocalSocket event delivery (direct + full provider chain) and a reentrant-release regression that drives release() from inside a deferred-completion callback. The hard crash only reproduces cross-process (in-process QtRO posts the event, so the read stack has already unwound) — the cross-process guard is the wallet Anvil integration doctest, where this fix is A/B-proven: the published backend crashes on refresh_balances, the patched backend returns balances cleanly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Dario LipicarandClaude Opus 4.8
|
4ea32a314a |
Per-module concurrent dispatch: async provider seam + transports (#5)
* feat: per-module concurrent dispatch (concurrency:"multi") — zero ABI change
A "multi" module serves calls concurrently behind the ORDINARY callMethod — no
new provider/host vtable method, so LogosProviderObject's ABI is byte-identical
to before and an old host/daemon loads and forwards a multi module unmodified.
Mechanism: a multi module's generated glue returns a pending sentinel
({"__logos_pending_call__": callId}) from callMethod and pushes the real result
back later as a __logos_call_complete__ event keyed by callId, over the existing
event channel. The consumer transport detects the sentinel and awaits the
completion transparently, so generated clients are unchanged.
- logos_async_dispatch.h: shared wire constants + the contract.
- remote_transport.cpp (QtRO) / plain_logos_object.{h,cpp} (plain): consumer
sentinel detection + await keyed by callId. The host is a pure forwarder.
- logos_protocol.h + nix/default.nix: protocol 0.2.0 (additive minor; same MAJOR
stays compatible, so an old host accepts a 0.2 "multi" module).
- rpc_server.cpp: fix a teardown self-deadlock (stop() held m_mu while invoking a
per-connection error handler that re-locks m_mu) that the new in-process
subscription path exposed.
- tests/protocol/test_concurrent_dispatch.cpp: proves a multi provider overlaps
two concurrent calls (peak 2) while single serializes (peak 1), over the plain
transport, with the host unchanged from master.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: coalesce concurrent async requestModule handshakes (+ async fan-out test)
A driver that fans out N async calls to an un-tokened target before any
completes used to fire N separate requestModule handshakes. Each mints a
distinct capability token and informs the target, and the later inform
OVERWRITES the earlier token there (the target stores one token per caller),
so the already-dispatched calls carried a superseded token and the target
rejected them as unauthorized ("auth token not recognized"). The sync path
never hit this — it blocks per call, so handshakes never overlap.
Coalesce in LogosAPIClient::invokeRemoteMethodAsync: the first async call to
an un-tokened target starts ONE handshake; concurrent calls to the same
target queue behind it and all drain with the single minted token when it
resolves. m_pendingHandshakes is touched only on the owner thread, so no lock
(appended last per the class's ABI note). This is what lets a concurrency:
"multi" worker actually run a single-threaded driver's fan-out concurrently —
otherwise the fanned-out calls are rejected before reaching dispatch.
Also add MultiProviderOverlapsAsync / SingleProviderSerializesAsync to the
concurrent-dispatch gtest: they fire N concurrent callMethodAsync() calls (the
fan-out pattern over the async consumer path, which the sync tests don't
exercise) and assert peak overlap 4 for "multi", 1 for "single".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Dario Lipicar
|
9de4165ab6 |
Qt split + module authoring groundwork: LogosProviderPlugin + the common module-impl C ABI (#3)
* Extract the Logos protocol layer from logos-cpp-sdk
Transports (plain TCP/TLS, qt_local, qt_remote/QRO, mock), token manager,
consumer core (LogosAPIClient/LogosAPIConsumer incl. the capability
auto-requestModule flow), ModuleProxy, the abstract LogosProviderObject
interface, and the canonical QVariant<->JSON conversion — now behind the
language-neutral lp_* C ABI (logos_protocol.h) carrying the protocol
semver (LOGOS_PROTOCOL_VERSION_*, lp_protocol_version()).
Bytes crossing the ABI use the lossless {"_bytes": base64url} tagging
(NUL-safe), matching the plain wire encoding.
Provider lp_* surface is compiled groundwork; serving lands with module
authoring.
* Move LogosProviderPlugin into logos_provider_interface.h
Plugin-loading tools (logos-cpp-generator's introspection mode, lm, the
hosts) need only qobject_cast<LogosProviderPlugin*>() + the abstract
LogosProviderObject — both framework-internal. Hosting the detection
interface here keeps those tools off the developer-facing logos-qt-sdk
layer. Same iid (org.logos.LogosProviderPlugin); header-only, ABI-neutral.
* Define the common module-impl C ABI (logos_module_impl.h)
ONE cdylib contract for module implementations in every language:
dispatch / get_methods / set_context / set_emit_callback / accept_token
/ get_protocol_version / string_free. The C++ and Rust SDKs emit these
exports around their respective impls; the uniform generated Qt glue
(and later a no-Qt host) talks to the cdylib only through this ABI.
JSON data model and tagged bytes form match the lp_* consumer ABI; the
protocol-version handshake complements the build-time metadata stamp.
* json convert: integers stay integers across the C ABI
QJsonValue::fromVariant degrades every numeric to double, so Int/UInt/
LongLong/ULongLong QVariants serialized as 5.0 — and a strict consumer on
the other side of the C ABI (a generated dispatch reading an int param)
rejects or zeroes them. Surfaced by the first cdylib-authored module
whose inbound args cross qvariantToNlohmann; the dlopen smoke harness
fed hand-written int JSON and never exercised this edge.
* call-error channel: surface {code,message,origin} for unacquirable targets
invokeRemoteMethod could not distinguish a failed call from a void/null
result — lp_invoke returned LP_OK with a null JSON result even when the
target module was never reached, and generated typed wrappers silently
defaulted (0 / empty string). Additive err-out overloads on
LogosAPIConsumer/LogosAPIClient fill a std-only logos::CallError
(logos_call_error.h, new LogosCallError exception for the generated
wrappers to throw); lp_invoke now honors its documented contract for
this class of failure: LP_ERR_UNAVAILABLE + canonical error JSON.
First detectable code: object_unavailable (requestObject failure) —
the struct is the extension point for transport-level statuses.
* call-error: drop the exception type — the error channel is the out-param
Per review, generated wrappers expose CallError as an optional trailing
out-parameter instead of throwing; the struct is the whole contract.
* ci: build + run the protocol test suite
On every pull request (unfiltered — stacked PRs included), master pushes,
and manual dispatch. The repo shipped without CI; its 111-test suite only
ran locally and through the workspace gate.
* consumer: typed requestModule for the capability flow
Port of logos-cpp-sdk master f5a127dd ('use updated capability module',
cpp-sdk#85, Iuri Matias) — the touched files (logos_api_client.cpp,
logos_api_consumer.{h,cpp}) moved into this repo in the P1 extraction.
The capability auto-requestModule path now calls a typed std::string
helper on the consumer (which acquires the capability object directly)
instead of a stringly invokeRemoteMethod round-trip. 111/111 tests.
* ci: DeterminateSystems nix installer (macOS runners)
cachix/install-nix-action fails on the macOS runners with
eDSRecordAlreadyExists (pre-existing nix build users); the org's
macOS-bearing workflows use the DeterminateSystems installer.
|
||
|
Dario Lipicar
|
29afbac532 |
Extract the Logos protocol layer from logos-cpp-sdk (lp_* C ABI + protocol semver) (#2)
* Extract the Logos protocol layer from logos-cpp-sdk
Transports (plain TCP/TLS, qt_local, qt_remote/QRO, mock), token manager,
consumer core (LogosAPIClient/LogosAPIConsumer incl. the capability
auto-requestModule flow), ModuleProxy, the abstract LogosProviderObject
interface, and the canonical QVariant<->JSON conversion — now behind the
language-neutral lp_* C ABI (logos_protocol.h) carrying the protocol
semver (LOGOS_PROTOCOL_VERSION_*, lp_protocol_version()).
Bytes crossing the ABI use the lossless {"_bytes": base64url} tagging
(NUL-safe), matching the plain wire encoding.
Provider lp_* surface is compiled groundwork; serving lands with module
authoring.
* consumer: typed requestModule for the capability flow
Port of logos-cpp-sdk master f5a127dd ('use updated capability module',
cpp-sdk#85, Iuri Matias) — the touched files (logos_api_client.cpp,
logos_api_consumer.{h,cpp}) moved into this repo in the P1 extraction.
The capability auto-requestModule path now calls a typed std::string
helper on the consumer (which acquires the capability object directly)
instead of a stringly invokeRemoteMethod round-trip. 111/111 tests.
|