net: SocketBackend::bindModel drops contextKey on a private registration, unlike SimulatedRemoteBackend

[Yaraslaut]

Surfaced while implementing #569 (PR #586); not folded into that PR per AGENTS.md, since it's a pre-existing defect the native bindModel path deliberately preserved rather than introduced.

The finding (inferred from reading the code, not yet reproduced)

SocketBackend does not override registerModelWithContext, so the default implementation forwards to registerModel and discards contextKey — while wire::makeRegister accepts a contextKey and SimulatedRemoteBackend does override the verb to carry it across.

Consequence: a server-side action log has no entity key for instances registered privately over morph::net.

bindModel's native path (added in #586) stays bit-for-bit identical to the blocking one on this point — the drop was deliberately preserved, with a comment in socket_backend.hpp saying why, rather than silently fixed as a drive-by.

Status

Inferred from reading include/morph/net/socket_backend.hpp against wire::makeRegister and SimulatedRemoteBackend's override (core/remote.hpp). Not reproduced against a running server / action log. Needs verification before a fix is written.

🤖 Generated with Claude Code

[Yaraslaut]

Triage: valid — premise confirmed, and the consequence is worse than stated

Assessed against master 70792bee (i.e. with #586 landed).

The drop is real, on both paths

The async path added by #586, include/morph/net/socket_backend.hpp:313:

if (request.primary.empty()) {
    // `contextKey` is dropped here because the blocking path drops it: ...
    return sendControlAsync(::morph::wire::makeRegister(request.typeId), "register", std::nullopt, cbExec);
}

wire::makeRegister does take the key — include/morph/core/wire.hpp:161:

inline Envelope makeRegister(std::string typeId, std::string contextKey = {}) {

so request.contextKey is discarded by omission at the call site. The blocking
path drops it too, for the reason the issue gives: SocketBackend never
overrides registerModelWithContext, so it inherits the default at
include/morph/core/backend.hpp:162:

    virtual ::morph::exec::detail::ModelId registerModelWithContext(
        const std::string& typeId, std::function<...> factory, std::string_view contextKey) {
        (void)contextKey;
        return registerModel(typeId, std::move(factory));
    }

SimulatedRemoteBackend does override it (core/remote.hpp:1849), building
makeRegister(typeId, std::string{contextKey}). So the asymmetry the issue
describes is exactly as reported.

The shared paths are unaffected — makeRegisterShared and makeAttach both
carry request.contextKey in the very same function. The gap is the private
registration only, as titled.

The consequence is stronger than the issue says

"a server-side action log has no entity key for instances registered privately over morph::net"

It is not a log missing a key. No log is attached at all, because the
provider is never consulted. RemoteServer::attachLogIfConfigured,
core/remote.hpp:704:

void attachLogIfConfigured(IModelHolder& holder, const wire::Envelope& env) {
    if (env.contextKey.empty()) {
        return;                       // <-- provider never looked up
    }
    ...
    if (auto log = provider(env.typeId, env.contextKey)) {
        holder.attachActionLog(std::move(log), env.contextKey);
    }
}

This is not my inference — the repository already has a passing test that
pins it, tests/test_action_log_phase2.cpp:347:

TEST_CASE("RemoteServer::setLogProvider: not consulted when contextKey is empty", ...) {
    ...
    auto reply = wire::decode(server->handleInline(wire::encode(wire::makeRegister("P2_Model"))));
    REQUIRE(reply.kind == "ok");
    REQUIRE_FALSE(providerCalled);

makeRegister("P2_Model") is byte-for-byte the envelope SocketBackend::bindModel
sends on the private path. The existing test therefore already demonstrates the
server-side half of this defect; it simply was not connected to the socket
backend when it was written. Its sibling at :308 shows the working case
(makeRegister("P2_Model", "acct-9") → entries[0].entityKey == "acct-9").

So the user-visible effect is: an instance registered privately over
morph::net is not journaled at all, while the same registration over
SimulatedRemoteBackend is. That is a silent divergence between two backends
that are supposed to be interchangeable, and it fails open (no audit record)
rather than closed.

Why valid rather than rescope

The fix direction is not a judgement call — the default's own doc comment
(backend.hpp:155) already states the rule:

"Backends whose model instances live behind a wire protocol
(SimulatedRemoteBackend) override this to carry @p contextKey across."

SocketBackend is a wire-protocol backend that did not override it. The design
intent is written down and SocketBackend simply does not meet it, so this is a
plain defect against a documented contract, not a framing problem. Both edges
need it, and they are small:

• async: makeRegister(request.typeId, request.contextKey) at socket_backend.hpp:318;
• blocking: an override of registerModelWithContext mirroring remote.hpp:1849.

Per the AGENTS.md "undoing a deliberate fix" check, git log -S on the call
site returns only 70792bee (#586) — the drop is not an old deliberate
decision, it is a knowingly-preserved pre-existing gap, and #586's comment says
so and points here. #586 was right not to fold it in.

Verification status

Reproduced at the server boundary; not reproduced end-to-end over a socket.

• Measured: nothing new. I read the four sites above and confirmed the existing
  tests at test_action_log_phase2.cpp:308/:347 encode both the working and
  the failing case. I did not stand up a SocketServer + SocketBackend
  pair and observe a missing journal over a real connection, so the transport
  half is inferred — though the transport cannot reintroduce a field the
  envelope was never given.
• Not measured: whether any shipped rung registers privately over morph::net
  with a non-empty contextKey today. If none does, this is latent rather
  than active, which affects priority but not validity.

What would close it

A test that registers privately through SocketBackend with a non-empty
contextKey against a RemoteServer with a log provider installed, and asserts
the provider was consulted with that key. Note per AGENTS.md "would this still
pass if the feature did nothing" — assert on providerCalled/entityKey, not
merely that registration succeeded, since registration succeeds today.

What would change the verdict

Close this if private registration over the wire is deliberately excluded from
journaling — but then SimulatedRemoteBackend's override is the bug, the two
backends still must not disagree, and backend.hpp:155 would need rewriting.

[Yaraslaut]

Reproduced end-to-end over a real socket — the transport half is no longer inferred

Both the issue body and the triage comment were explicit that this had not been reproduced past the server boundary. It has been now. PR #595.

Setup: a real SocketServer in front of a RemoteServer with a LogProvider installed, and a real SocketBackend client over a real TCP connection — not handleInline. Measured on Clang 22.1.8, Debug, -DMORPH_BUILD_NET=ON, with the new test applied and include/morph/net/socket_backend.hpp still at its 992b190c state:

tests/net/test_socket_backend.cpp:2191: FAILED:
  CHECK( requestedFor == std::vector<std::string>{"SbEchoModel:acct-587"} )
with expansion:
  {  } == { "SbEchoModel:acct-587" }

tests/net/test_socket_backend.cpp:2218: FAILED:
  REQUIRE( entries.size() == 1 )
with expansion:
  0 == 1

tests/net/test_socket_backend.cpp:2227: FAILED:
  CHECK( requestedFor == std::vector<std::string>{"SbEchoModel:acct-blocking"} )
with expansion:
  {  } == { "SbEchoModel:acct-blocking" }

test cases:  1 |  0 passed | 1 failed
assertions: 15 | 12 passed | 3 failed

With the fix:

All tests passed (17 assertions in 1 test case)

Three things the measurement adds to the triage's reading:

1. The triage's stronger reading was right. The middle failure is the one that matters — entries.size() == 1 expanding to 0 == 1. The provider is not merely handed an empty key; it is never consulted, so no log is attached and the instance produces no journal entry for an action it did execute. Confirmed against a real transport, not just attachLogIfConfigured's early return.
2. Both edges fail independently. The first two failures are the native bindModel path, the third the blocking registerModelWithContext path, in separate SECTIONs. Neither masks the other, so neither half of the fix is unverified.
3. The mutation check the triage asked for holds. Registration itself succeeded in every run above, before and after. Nothing in the test asserts on it; every assertion is on requestedFor or the entry's entityKey. A third section pins the negative — plain registerModel still sends no key and the provider is still not consulted — so the fix did not just start stamping a key onto everything.

Also within scope, and worth recording: registerModelShared and attachModel degrade to registerModelWithContext when primary is empty. Those degrades are private registrations too and dropped the key for the same reason; they are fixed by the same override. The issue titled only bindModel, which understated the surface slightly in the same direction the triage noted for the consequence.

Still not measured, unchanged from the triage: whether any shipped rung registers privately over morph::net with a non-empty contextKey today. That affects priority, not validity.

Filed separately, not folded: #594 — QtWebSocketBackend has the same gap on its blocking private registration and disagrees with its own async path, which does carry the key. Because asyncRegistrationEnabled defaults to false, the default configuration of the Qt transport takes the dropping path. Filed as inferred from reading the code, not reproduced.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GS5K2vqZtC4xbRiGJHT7jH