`dispatchRejection` matched the single literal "dispatch_failed", so the
refusal `logosctl call` runs into most often went straight through it. Providers
answer a wrong argument COUNT with "invalid_args" — logos-cpp-sdk's cdylib
dispatch and logos-rust-sdk's args::invalid_args both do, and always have — and
no detector anywhere matched it. Measured on this very command:
logosctl call test_basic_module isPositive (missing required argument)
-> exit 0, status:"ok",
result {"code":"invalid_args","message":"expected 1 arguments, got 0",
"origin":"test_basic_module"}
An arity error reported as a successful call that returned a map. It now reports
METHOD_FAILED with error.code "invalid_args", and `logosctl call` exits 4
instead of 0 (call_command.cpp maps any status:"error" that is not a
MODULE_NOT_* to 4).
The parent commit made the envelope read the error CHANNEL instead of judging by
the value; this makes the in-band half of the same split as complete. Both are
one question — "did this call succeed?" — and answering it from one of the two
places was the whole defect.
`code` is now matched against a CLOSED SET, in one named array so it cannot
drift from the rest of the function: dispatch_failed, invalid_args,
unknown_method. Nothing emits "unknown_method" yet and it is listed anyway,
because a detector can be widened compatibly on its own while a new provider
code cannot — one shipped against narrow detectors would arrive as data.
Closed, not open. A method may legitimately return a {code, message, origin}
map of its own, so matching the SHAPE would turn its data into an error. The
three guards (exactly three keys, all present, all strings) are untouched, and
the tests now pin that as behaviour: eight unrecognised codes including
"DISPATCH_FAILED", "dispatch_failed " and "invalid_argument", 2- and 4-key
objects, and a non-string in each of the three slots all stay DATA — plus an
end-to-end case where an unrecognised three-string map comes back as "result".
ONE THING LEFT DELIBERATELY UNDONE, recorded at the declaration: when a provider
eventually emits "unknown_method", it will fold to METHOD_FAILED here rather
than to the METHOD_NOT_FOUND envelope callEnvelope already builds from
introspection. Choosing between those two belongs with the provider-contract
change, not with widening a detector, and any routing written now would be
untested against a real provider.
Verified: 13/13 test_call_envelope, and a negative control — the two new
positive tests FAIL against the pre-change detector and pass against this one,
while the narrowness tests pass against both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
callModuleMethod judged failure with `ret.is_null()` because it called the
one invokeRemoteMethod overload that has no CallError* parameter
(logos_api_client.h:352-356, whose body forwards to the QVariant overload
with the error channel dropped). A method that legitimately returns null was
therefore indistinguishable from a call that failed — and that single line
was the entire empirical basis for the qt-generator's refusal to allow an
optional return.
Switching to the CallError-carrying overload (logos_api_client.h:98) is not
sufficient on its own: an unknown method name is deliberately NOT reportable
on the wire (logos_protocol.h:274-279 says so outright, and the cdylib
dispatch ends `return nullptr; // unknown method`), so a naive !err.ok()
would have turned every typo into a silent success. The decision is now:
!err.ok() -> METHOD_FAILED + {code,message,origin}
result is a dispatch_failed envelope -> METHOD_FAILED (the provider refused)
null AND method provably not exposed -> METHOD_NOT_FOUND + available_methods
otherwise -> ok, null included
METHOD_NOT_FOUND is not invented — docs/spec.md:918 specified that envelope,
with available_methods, all along; core_service simply never produced it. It
costs one extra round-trip only on a null return.
The logic lives in a new pure unit, core_service/call_envelope.{h,cpp}, with
no Qt and no logos-protocol, which is what makes it unit-testable at all. The
value path is byte-identical: the same nlohmannArgsToQVariantList /
qvariantToNlohmann the json overload used internally.
Behaviour changes a reviewer must agree with: a null return is now `ok`
rather than METHOD_FAILED, and a dispatch_failed envelope returned as data is
now METHOD_FAILED rather than `ok`. No existing test encoded the old
behaviour; no exit code or ok/error verdict flipped in any fixture.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
