net: carry contextKey on SocketBackend's private registration (fixes #587)

CHANGELOG.md

@@ -182,6 +182,25 @@ API surface).
 
 ### Fixed
 
+- **A model registered privately over `morph::net` was not journalled at all.**
+  `morph::net::SocketBackend` left `IBackend::registerModelWithContext`
+  unoverridden, so its default dropped the `contextKey`, and the native
+  `bindModel` path added in morph#586 dropped it again by omission at the
+  `wire::makeRegister` call site. Because `RemoteServer::attachLogIfConfigured`
+  returns *without consulting its `LogProvider`* when the envelope's
+  `contextKey` is empty, the effect was not a log entry missing its entity
+  key — no log was attached, so the instance produced no audit record at all,
+  while the same registration over `SimulatedRemoteBackend` produced one. It
+  failed open. `SocketBackend` now overrides `registerModelWithContext` and
+  passes `request.contextKey` to `makeRegister` on the `bindModel` private
+  branch, so both edges carry the key; the empty-`primary` degrades of
+  `registerModelShared`/`attachModel`, which route through
+  `registerModelWithContext`, are fixed with them. The shared and attach shapes
+  already carried it and are unchanged. Reproduced end-to-end over a real
+  socket in `tests/net/test_socket_backend.cpp`, which asserts the provider was
+  consulted with the key and that the attached log records the action under it.
+  See morph#587.
+
 - **A locale-formatted entry could submit ten times what the user typed.**
   `morph::render::normalizeLocaleNumber` dropped every occurrence of the group
   separator unconditionally, with no check on placement, so a de-DE user typing

docs/spec/core/backend.md

@@ -75,7 +75,7 @@ holds a `unique_ptr<IBackend>` and delegates all model operations to it.
 | Method | Purpose |
 |---|---|
 | `registerModel(typeId, factory)` | Registers a new model instance, returns its opaque `ModelId`. |
-| `registerModelWithContext(typeId, factory, contextKey)` | Same as `registerModel`, additionally passes a stable identity (e.g. account id). Default implementation drops `contextKey` and forwards to `registerModel` — correct for `LocalBackend` where the factory closure already captures identity. `SimulatedRemoteBackend` overrides to carry `contextKey` across the wire. |
+| `registerModelWithContext(typeId, factory, contextKey)` | Same as `registerModel`, additionally passes a stable identity (e.g. account id). Default implementation drops `contextKey` and forwards to `registerModel` — correct for `LocalBackend` where the factory closure already captures identity. Every backend whose instances live behind a wire protocol overrides it to carry `contextKey` across: `SimulatedRemoteBackend` and `SocketBackend` both do. Not cosmetic — `RemoteServer::attachLogIfConfigured` skips the `LogProvider` lookup entirely on an empty `contextKey`, so a wire backend that drops the key leaves the instance with **no** action log rather than a log missing a field (morph#587). |
 | `registerModelAsync(typeId, factory, contextKey, onRegistered, onError)` | Optional non-blocking counterpart to `registerModelWithContext`. Returns `false` by default (no async path); `Bridge::registerHandler()` prefers this when it returns `true` and falls back to the synchronous call otherwise. See [Asynchronous registration](#asynchronous-registration--registermodelasync). |
 | `bindModel(request, cbExec)` | Acquires a model instance and returns a `Completion<ModelId>` delivered on `cbExec`. One verb covering `registerModelWithContext`, `registerModelShared` and `attachModel`, selected by the request's shape. The preferred surface — see [The structural registration surface](#the-structural-registration-surface--bindmodel-and-promotemodel). |
 | `promoteModel(request, cbExec)` | Files an already-live instance under a key and returns a `Completion<ModelId>` delivered on `cbExec`. The structural counterpart of `assignPrimary`. |
@@ -2062,8 +2062,9 @@ not a behavior change to the existing loopback-only default.
 |---|---|
 | `explicit SocketBackend(serverUrl, cfg = Config{})` | Parses `serverUrl` (`ws://` only — throws immediately on `wss://`) and starts the I/O thread, which connects asynchronously. |
 | `waitForConnected(timeout = 5000ms)` | Blocks the calling thread on a condition variable until connected or the timeout elapses; returns the current connected state. The backend must outlive the call — destroying it while a thread is parked here is undefined, and there is no cancel (see Lifetime & ownership). |
-| `registerModel(typeId, factory)` | Synchronous via a parked condition variable; `factory` ignored. Throws on `err` reply or disconnect. Thread-safe, but only one such call may be in flight at a time. |
-| `bindModel(request, cbExec)` | Native override of the structural surface. Sends the envelope `request`'s shape names with a non-zero `callId` and returns immediately; the I/O thread settles the `Completion` when the reply arrives, delivered on `cbExec`. Never enters `sendSync`, so it takes no synchronous-call token and any number may be in flight. Rejects with `DisconnectedError` when the socket is down or drops first, or with `std::runtime_error{"<verb> failed: <server message>"}`. |
+| `registerModel(typeId, factory)` | Forwards to `registerModelWithContext` with an empty `contextKey`; `factory` ignored. |
+| `registerModelWithContext(typeId, factory, contextKey)` | Synchronous via a parked condition variable; sends `register` carrying `contextKey`, so the server's `LogProvider` is consulted for a private registration exactly as it is for a shared one (morph#587). `factory` ignored. Throws on `err` reply or disconnect. Thread-safe, but only one such call may be in flight at a time. `registerModelShared` and `attachModel` degrade here when `primary` is empty, so their private paths carry the key too. |
+| `bindModel(request, cbExec)` | Native override of the structural surface. Sends the envelope `request`'s shape names with a non-zero `callId` and returns immediately; every shape carries `request.contextKey`, the private one included; the I/O thread settles the `Completion` when the reply arrives, delivered on `cbExec`. Never enters `sendSync`, so it takes no synchronous-call token and any number may be in flight. Rejects with `DisconnectedError` when the socket is down or drops first, or with `std::runtime_error{"<verb> failed: <server message>"}`. |
 | `promoteModel(request, cbExec)` | The `assign` counterpart of `bindModel`, on the same path; resolves with `request.mid` echoed back. An empty `primary` or a zero `mid` resolves without sending, matching `assignPrimary`'s guards. |
 | `deregisterModel(mid)` | **Fire-and-forget** — sends only if connected, does not wait for the ack. Carries a non-zero `callId` from the same counter `execute` uses so its unawaited `ok` cannot be handed to a parked synchronous control call (issue #454; the `QtWebSocketBackend` precedent is issue #65). Needs no pending-id bookkeeping of its own: `dispatchIncomingEnvelope` already drops a non-zero `callId` that is absent from `_pending`. |
 | `execute(mid, call, cbExec)` | Assigns a `callId`, sends `execute`, returns a `Completion`. Immediate `DisconnectedError` if not connected. Thread-safe; supports concurrent in-flight calls from multiple threads. |
@@ -2091,7 +2092,7 @@ not a behavior change to the existing loopback-only default.
 | Decision | Choice | Why |
 |---|---|---|
 | Dual-path `ActionCall` | Three callables: `localOp`, `serializeAction`, `deserializeResult` | The same `ActionCall` struct works for both local and remote execution without an `if (isRemote)` branch at the call site — each backend uses the field(s) it needs. |
-| `registerModelWithContext` | Virtual with a default that drops `contextKey` | `LocalBackend`'s factory closure already captures identity, so there is nothing to forward. `SimulatedRemoteBackend` overrides to carry `contextKey` across the wire so the server's `LogProvider` can attach an action log. |
+| `registerModelWithContext` | Virtual with a default that drops `contextKey` | `LocalBackend`'s factory closure already captures identity, so there is nothing to forward — which is why the default drops the key rather than being pure virtual. A backend whose instances are constructed on the far side of a wire protocol has no such closure, so the envelope is the only channel the identity has: `SimulatedRemoteBackend` and `SocketBackend` both override it so the server's `LogProvider` can attach an action log. The default being *permissive* is what let `SocketBackend` ship without an override and silently stop journalling private registrations (morph#587); the price of that permissiveness is that "is this a wire backend?" has to be answered by hand for each new transport. |
 | `RemoteServer` heap requirement | `std::enable_shared_from_this` | `handle()` posts to the worker pool capturing `shared_from_this()` — the server must outlive any in-flight message. |
 | `handleInline` | Synchronous; caller-restricted to control messages | Safe to call from a worker-pool thread (e.g. from a `BridgeHandler` constructor). It is meant for `register`/`deregister` only; an `execute` envelope is rejected with an `err` reply, because `dispatchExecute` posts to the strand and would reply after `handleInline` returns (writing into an already-destroyed reply buffer). The rejection is now enforced by the code, matching the documented intent. |
 | `SimulatedRemoteBackend` factory ignored | Model construction delegated to `RemoteServer`'s `ModelRegistryFactory` | The factory closure lives on the client side; the server owns the actual instances. |

include/morph/net/socket_backend.hpp

@@ -183,25 +183,42 @@ class SocketBackend : public ::morph::backend::detail::IBackend {
     /// is outstanding throws immediately rather than queuing. The factory
     /// argument is ignored — model construction is delegated to the server.
     /// @param typeId  String type-id of the model to register.
+    /// @param factory Ignored — the server constructs via its own registry.
     /// @return `ModelId` assigned by the server.
     /// @throws std::runtime_error if the server replies with an error, the
     ///         socket is not connected, or a synchronous call is already in flight.
     ::morph::exec::detail::ModelId registerModel(
         const std::string& typeId,
-        std::function<std::unique_ptr<::morph::model::detail::IModelHolder>()> /*factory*/) override {
-        auto env = ::morph::wire::makeRegister(typeId);
+        std::function<std::unique_ptr<::morph::model::detail::IModelHolder>()> factory) override {
+        return registerModelWithContext(typeId, std::move(factory), {});
+    }
+
+    /// @brief Sends a `register` message carrying @p contextKey and blocks for the reply.
+    ///
+    /// `IBackend::registerModelWithContext`'s default drops @p contextKey, which
+    /// is right for `LocalBackend` — the caller's own factory closure already
+    /// captures the identity — but wrong for a backend whose instances live on
+    /// the far side of a wire protocol: the server constructs the holder itself,
+    /// so `contextKey` is the *only* channel by which the instance's identity
+    /// reaches it. `RemoteServer::attachLogIfConfigured` returns without
+    /// consulting its `LogProvider` at all when the envelope's `contextKey` is
+    /// empty, so dropping it here does not merely lose an entity key — it leaves
+    /// the instance unjournalled (morph#587). `SimulatedRemoteBackend` overrides
+    /// this for the same reason; the two must not disagree.
+    ///
+    /// Same synchronous-call constraint as `registerModel`. The factory argument
+    /// is ignored — model construction is delegated to the server.
+    /// @param typeId     String type-id of the model to register.
+    /// @param contextKey Stable identity of the new instance; empty if none.
+    /// @return `ModelId` assigned by the server.
+    /// @throws std::runtime_error if the server replies with an error, the
+    ///         socket is not connected, or a synchronous call is already in flight.
+    ::morph::exec::detail::ModelId registerModelWithContext(
+        const std::string& typeId, std::function<std::unique_ptr<::morph::model::detail::IModelHolder>()> /*factory*/,
+        std::string_view contextKey) override {
+        auto env = ::morph::wire::makeRegister(typeId, std::string{contextKey});
         env.session = currentSession();
-        std::string replyJson;
-        try {
-            replyJson = sendSync(::morph::wire::encode(env));
-        } catch (const std::exception& exc) {
-            throw std::runtime_error(std::string{"register failed: "} + exc.what());
-        }
-        auto reply = ::morph::wire::decode(replyJson);
-        if (reply.kind == "ok") {
-            return ::morph::exec::detail::ModelId{reply.modelId};
-        }
-        throw std::runtime_error("register failed: " + reply.message);
+        return sendControlForId(env, "register");
     }
 
     /// @brief Sends a shared (register-or-attach) `register` and blocks for the reply.
@@ -310,12 +327,8 @@ class SocketBackend : public ::morph::backend::detail::IBackend {
             deregisterModel(request.current);
         }
         if (request.primary.empty()) {
-            // `contextKey` is dropped here because the blocking path drops it:
-            // `IBackend::registerModelWithContext`'s default forwards to
-            // `registerModel` and discards it, and this backend does not
-            // override it. Keeping the native path bit-for-bit identical
-            // matters more than changing that here; it is filed separately.
-            return sendControlAsync(::morph::wire::makeRegister(request.typeId), "register", std::nullopt, cbExec);
+            return sendControlAsync(::morph::wire::makeRegister(request.typeId, request.contextKey), "register",
+                                    std::nullopt, cbExec);
         }
         if (request.current.v != 0U) {
             return sendControlAsync(

scripts/branch_partial_allowlist.json

@@ -146,7 +146,7 @@
     },
     {
       "file": "include/morph/net/socket_backend.hpp",
-      "line": 813,
+      "line": 826,
       "source": "default:",
       "reason": "Unreachable except via the adjacent `Error` case it deliberately shares a body with (net audit, `socket_backend.hpp` extra finding #7). `detail::ExecuteReplyKind` is a closed 3-value enum (`Value`/`Timeout`/`Error`), all three handled explicitly above this label; `default:` exists only to satisfy this project's `-Wswitch-default`, per the source's own inline comment directly below this line. Reaching it via any value other than through the `Error` case falling through would require an out-of-range `static_cast` producing a value outside the enum's domain -- undefined behavior, not a legitimate test target."
     }