From d3ae249ec4e5ac1ff0a0750a3c3a2cfe9f0667fe Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 17 Aug 2026 00:40:02 +0300 Subject: [PATCH 1/3] core: IModelHolder::attachActionLog forwards to a model-level attachActionLog RemoteServer::LogProvider's attach path (attachLogIfConfigured) only ever populated the type-erased holder's own _actionLog/_contextKey, used by recordIfAttached's auto-append -- never a model instance's own state. A model that keeps its own model-level IActionLog reference to read its history back later (e.g. an activity-stream view over entries(entityKey)) had no way to receive the same log a registry-constructed, remote/keyed attach populates the holder with, since such an instance is always default-constructed and never otherwise touched. Adds IModelHolder::onActionLogAttached, a protected virtual hook (no-op default) that attachActionLog now calls before storing its own state. ModelHolder overrides it to forward to Model::attachActionLog(log, contextKey) when Model structurally satisfies the new ModelLevelActionLogAttachable concept -- the same "detect the hook structurally, forward only if present" shape onBackendChanged()/ BackendChangedMixin already use, so a model with no attachActionLog of its own is entirely unaffected. Updates docs/spec/core/registry.md and docs/spec/journal/journal.md to document the new hook. Verified with a standalone probe program exercising ModelHolder in isolation, and end-to-end in the kanban App bootstrap (next commit) where a registry-constructed BoardModel's GetActivity now sees entries the same dispatch produced. Co-Authored-By: Claude Sonnet 5 --- docs/spec/core/registry.md | 18 ++++++++- docs/spec/journal/journal.md | 16 ++++++++ include/morph/core/model.hpp | 77 ++++++++++++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/docs/spec/core/registry.md b/docs/spec/core/registry.md index 164940a8..9693067b 100644 --- a/docs/spec/core/registry.md +++ b/docs/spec/core/registry.md @@ -339,7 +339,20 @@ struct IModelHolder { - `into()` down-casts to a concrete `Model&`; throws `std::bad_cast` on mismatch. - `attachActionLog` sets the durable log sink and the instance's stable identity - (stamped onto every `LogEntry`). + (stamped onto every `LogEntry`), then calls the protected virtual + `onActionLogAttached(log, contextKey)` (base default: no-op) before storing + either. `ModelHolder` overrides this to forward to + `Model::attachActionLog(log, contextKey)` when `Model` structurally + satisfies `ModelLevelActionLogAttachable` (`morph/core/model.hpp`) — the + same "detect the hook structurally, forward only if present" shape + `onBackendChanged()`/`BackendChangedMixin` use below. This is what lets a + model that keeps its own model-level `IActionLog` reference (to read its + own history back later, e.g. an activity-stream view) receive the same log + instance a registry-constructed, remote/keyed attach populates the holder + with — see [journal.md's "Attaching a log to remote + instances"](../journal/journal.md#attaching-a-log-to-remote-instances). A + model with no `attachActionLog` of its own is unaffected: the hook resolves + to the base's no-op body for it. - `recordIfAttached` is called automatically by `ActionDispatcher`'s runner and `Bridge::executeVia` — model code never calls it directly. It fills `entityKey`, `principal` (from `session::current()`), and `timestampMs` on the @@ -363,6 +376,9 @@ struct ModelHolder : IModelHolder, BackendChangedMixin { std::type_index type() const noexcept override; bool isBackendChangeAware() const noexcept override; void onBackendChanged() override; + protected: + void onActionLogAttached(const std::shared_ptr<::morph::journal::IActionLog>&, + const std::string& contextKey) override; }; ``` diff --git a/docs/spec/journal/journal.md b/docs/spec/journal/journal.md index 8823eea2..9efa8285 100644 --- a/docs/spec/journal/journal.md +++ b/docs/spec/journal/journal.md @@ -544,6 +544,22 @@ void RemoteServer::setLogProvider(LogProvider provider); // thread-safe returns `nullptr` also attaches no log; otherwise the returned sink is attached via `holder->attachActionLog(log, contextKey)`, so `contextKey` becomes the entry `entityKey`. +- **Also reaches a model-level `attachActionLog`, if the model declares one.** + `IModelHolder::attachActionLog` forwards to a protected virtual hook, + `onActionLogAttached`, which `ModelHolder` overrides to call + `Model::attachActionLog(log, contextKey)` when `Model` structurally satisfies + `ModelLevelActionLogAttachable` (`morph/core/model.hpp`) — the same + "detect the hook structurally, forward only if present" shape + `onBackendChanged()`/`BackendChangedMixin` already use. This closes a real + gap for a model that keeps its own model-level `IActionLog` reference to + read back later (e.g. an activity-stream view over `entries(entityKey)`): + before this hook existed, `holder->attachActionLog(...)` populated only the + holder's own `_actionLog`/`_contextKey` (used by `recordIfAttached`'s + auto-append), never a model instance's own state, since a registry- + constructed model is always default-constructed and never otherwise + touched. A model with no `attachActionLog` of its own is unaffected — the + hook resolves to a no-op for it, exactly as before this existed. First + exercised by `kanban::BoardModel` (rung 4). - **Installing / removing.** `setLogProvider(nullptr)` removes a previously installed provider (subsequent registrations get no log). The provider slot is guarded by its own mutex, and the provider is copied out under that lock before diff --git a/include/morph/core/model.hpp b/include/morph/core/model.hpp index 18121d01..3bfbe239 100644 --- a/include/morph/core/model.hpp +++ b/include/morph/core/model.hpp @@ -41,6 +41,27 @@ concept BackendChangedNotifiable = requires(M& model) { { model.onBackendChanged() } -> std::same_as; }; +/// @brief Concept satisfied by model types that expose a model-level +/// `attachActionLog(std::shared_ptr, std::string)`. +/// +/// A small number of keyed/shared models (e.g. `kanban::BoardModel`) cannot +/// rely on `IModelHolder::recordIfAttached`'s auto-append alone (that +/// mechanism serves plain fire-after-success journaling; a model that needs +/// to read its own log back, e.g. for an activity-stream view, needs the +/// *same* `IActionLog` instance available on `Model` itself, not only on the +/// type-erased holder wrapping it) and so declares its own `attachActionLog` +/// method with this exact shape, purely by structural convention — there is +/// no shared base class or interface a model opts into. This concept is what +/// lets `ModelHolder::onActionLogAttached` forward to it generically, +/// the same "detect the hook structurally, forward only if present" shape +/// `BackendChangedNotifiable`/`BackendChangedMixin` already establish for +/// `onBackendChanged()`. +template +concept ModelLevelActionLogAttachable = requires(M& model, std::shared_ptr<::morph::journal::IActionLog> log, + std::string key) { + { model.attachActionLog(log, key) } -> std::same_as; +}; + // ── Conditional mixin ───────────────────────────────────────────────────────── /// @brief Empty base when `M` does not declare `onBackendChanged()`. @@ -99,13 +120,23 @@ struct IModelHolder { /// @brief Attaches a durable action log and this instance's stable identity. /// /// Set once, typically from the same custom `HandlerBinding::modelFactory` - /// closure already used to inject other dependencies. @p contextKey is stamped - /// onto every `LogEntry` this instance produces (e.g. an account id) so log - /// entries are identifiable without parsing `payload`/`result` JSON. + /// closure already used to inject other dependencies, or from + /// `RemoteServer::LogProvider` for a registry-constructed remote instance + /// (see `docs/spec/journal/journal.md`, "Attaching a log to remote + /// instances"). @p contextKey is stamped onto every `LogEntry` this + /// instance produces (e.g. an account id) so log entries are identifiable + /// without parsing `payload`/`result` JSON. + /// + /// Also forwards @p log/@p contextKey to the wrapped model's own + /// `attachActionLog(log, contextKey)`, if it declares one matching + /// `ModelLevelActionLogAttachable` — see `onActionLogAttached`'s doc + /// comment for why this second call exists alongside the holder's own + /// `_actionLog`/`_contextKey` state below. /// @param log Sink entries are forwarded to. Pass a `SessionLog` to also /// get undo/checkpoint support. /// @param contextKey Stable identity of this model instance. void attachActionLog(std::shared_ptr<::morph::journal::IActionLog> log, std::string contextKey) { + onActionLogAttached(log, contextKey); _actionLog = std::move(log); _contextKey = std::move(contextKey); } @@ -156,6 +187,36 @@ struct IModelHolder { _actionLog->append(std::move(entry)); } +protected: + /// @brief Forwards @p log/@p contextKey to the wrapped model's own + /// `attachActionLog`, if it declares one. No-op default, for the + /// overwhelming majority of models that only ever rely on + /// `recordIfAttached`'s auto-append and have no model-level state + /// of their own to keep a log reference in. + /// + /// `ModelHolder` overrides this exactly like `onBackendChanged()` + /// (see `BackendChangedMixin`): detected structurally via + /// `ModelLevelActionLogAttachable`, not through a shared + /// interface a model opts into by inheritance. This closes the gap a + /// registry-constructed remote instance would otherwise have — before + /// this hook existed, `RemoteServer::LogProvider`'s `holder-> + /// attachActionLog(log, contextKey)` populated only this holder's own + /// `_actionLog`/`_contextKey` (used by `recordIfAttached`'s auto-append), + /// never a `Model`-level `_log`-shaped member a model reads back from + /// itself (e.g. for an activity-stream view): the model instance + /// `into()` down-casts to is default-constructed by `ModelFactory:: + /// create()` and never otherwise touched. A model with no + /// `attachActionLog` of its own is entirely unaffected — this call + /// resolves to the base's no-op body for it, same as before this hook + /// was added. + /// @param log Sink entries are forwarded to. + /// @param contextKey Stable identity of this model instance. + virtual void onActionLogAttached(const std::shared_ptr<::morph::journal::IActionLog>& log, + const std::string& contextKey) { + (void) log; + (void) contextKey; + } + private: std::shared_ptr<::morph::journal::IActionLog> _actionLog; std::string _contextKey; @@ -196,6 +257,16 @@ struct ModelHolder : IModelHolder, BackendChangedMixin { model.onBackendChanged(); } } + + protected: + /// @brief Forwards to `Model::attachActionLog(log, contextKey)` iff `Model` + /// declared one matching `ModelLevelActionLogAttachable`; no-op otherwise. + void onActionLogAttached(const std::shared_ptr<::morph::journal::IActionLog>& log, + const std::string& contextKey) override { + if constexpr (ModelLevelActionLogAttachable) { + model.attachActionLog(log, contextKey); + } + } }; /// @brief Factory that creates default-constructed `ModelHolder` instances. From e6d6388210d8d672827ccaa3610bc0ae571c36d0 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 17 Aug 2026 12:27:32 +0300 Subject: [PATCH 2/3] qt+net: QtWebSocketBackend/SocketBackend::listInstances never stamped the session Both classes' listInstances() built the wire envelope via morph::wire::makeInstances(typeId) and sent it directly, unlike every other envelope-building call site in the same classes (register, attach, assign, execute, and deregister all set env.session before sending). RemoteServer's instances handler authorizes with IAuthorizer::authorize(env.session, typeId, {}) -- with no session on the envelope, a SigningAuthorizer-derived authorizer (or any authorizer that actually inspects the session) rejects the call as unauthorized, even on a connection that already completed a correctly-authenticated execute(). Every existing rung's instances() coverage used an AllowAllAuthorizer- derived authorizer (or ran Local/in-process, where authorizeInstance never runs at all per docs/spec/security.md), so authorize() was always permissive regardless of session and this path went unexercised. Fixed by stamping env.session from currentSession() (SocketBackend) / _session (QtWebSocketBackend) before sending, matching every sibling call site. Found while adding a rung's first Socket-mode instances() test against a SigningAuthorizer-derived authorizer. Co-Authored-By: Claude Sonnet 5 --- include/morph/net/socket_backend.hpp | 4 +++- src/qt/qt_websocket_backend.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/morph/net/socket_backend.hpp b/include/morph/net/socket_backend.hpp index 7141f07d..6f7993b9 100644 --- a/include/morph/net/socket_backend.hpp +++ b/include/morph/net/socket_backend.hpp @@ -211,7 +211,9 @@ class SocketBackend : public ::morph::backend::detail::IBackend { std::vector listInstances(const std::string& typeId) override { std::string replyJson; try { - replyJson = sendSync(::morph::wire::encode(::morph::wire::makeInstances(typeId))); + auto env = ::morph::wire::makeInstances(typeId); + env.session = currentSession(); + replyJson = sendSync(::morph::wire::encode(env)); } catch (const std::exception& exc) { throw std::runtime_error(std::string{"instances failed: "} + exc.what()); } diff --git a/src/qt/qt_websocket_backend.cpp b/src/qt/qt_websocket_backend.cpp index 850222e5..85ca6177 100644 --- a/src/qt/qt_websocket_backend.cpp +++ b/src/qt/qt_websocket_backend.cpp @@ -381,7 +381,9 @@ bool QtWebSocketBackend::assignPrimaryAsync(::morph::exec::detail::ModelId mid, } std::vector QtWebSocketBackend::listInstances(const std::string& typeId) { - auto reply = ::morph::wire::decode(sendSync(::morph::wire::encode(::morph::wire::makeInstances(typeId)))); + auto env = ::morph::wire::makeInstances(typeId); + env.session = _session; + auto reply = ::morph::wire::decode(sendSync(::morph::wire::encode(env))); if (reply.kind != "ok") { throw std::runtime_error("instances failed: " + reply.message); } From 2e4d8e4ae333beb4496131a3d6b86b3b7cc655aa Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 17 Aug 2026 12:56:57 +0300 Subject: [PATCH 3/3] tests: cover ModelHolder::onActionLogAttached's forwarding branch Every existing attachActionLog test used ALModel, which has no model-level attachActionLog of its own -- so all of them exercised only the ModelLevelActionLogAttachable == false arm of ModelHolder:: onActionLogAttached's `if constexpr`. The == true arm (the actual forwarding mechanism this hook exists for) had no coverage at all on this branch, since the only model in the tree that declares a matching attachActionLog is kanban::BoardModel, which lives on a separate branch. Adds ALLoggingModel, a minimal model declaring attachActionLog matching ModelLevelActionLogAttachable's exact shape, plus two tests: - The forwarding case: attachActionLog on the holder reaches the wrapped model's own attachActionLog with the same log/contextKey, exactly once, without disturbing the holder's own hasActionLog/recordIfAttached state. - The no-op case (ALModel): attachActionLog still succeeds and populates the holder's own state when the wrapped model has no attachActionLog of its own -- confirming the hook's default body runs harmlessly for the overwhelming majority of models that don't opt in. Co-Authored-By: Claude Sonnet 5 --- tests/test_action_log.cpp | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/test_action_log.cpp b/tests/test_action_log.cpp index 008fac47..c45433b4 100644 --- a/tests/test_action_log.cpp +++ b/tests/test_action_log.cpp @@ -222,6 +222,73 @@ TEST_CASE("IModelHolder: attachActionLog stamps entityKey and timestamp automati REQUIRE(entries[0].timestampMs > 0); } +// ── ModelHolder::onActionLogAttached forwarding ────────────────────── +// +// ALModel above has no model-level attachActionLog of its own, so every test +// above exercises only the ModelLevelActionLogAttachable == false arm +// of ModelHolder::onActionLogAttached's `if constexpr`. ALLoggingModel +// declares one matching the concept's exact shape, to exercise the == true +// arm: IModelHolder::attachActionLog forwarding log/contextKey to the +// wrapped model's own attachActionLog, the mechanism a keyed/shared model +// (e.g. kanban::BoardModel) needs to read its own journal back for an +// activity-stream-shaped view, since the type-erased holder's _actionLog is +// otherwise never visible to Model::execute() itself. + +struct ALLoggingDeposit { + int amount = 0; +}; + +struct ALLoggingModel { + std::shared_ptr log; + std::string contextKey; + int callCount = 0; + + void attachActionLog(std::shared_ptr attachedLog, std::string attachedContextKey) { + log = std::move(attachedLog); + contextKey = std::move(attachedContextKey); + ++callCount; + } + + int execute(const ALLoggingDeposit& a) { return a.amount; } +}; + +BRIDGE_REGISTER_MODEL(ALLoggingModel, "AL_LoggingModel") +BRIDGE_REGISTER_ACTION(ALLoggingModel, ALLoggingDeposit, "AL_LoggingDeposit") + +TEST_CASE("ModelHolder::onActionLogAttached forwards to a model-level attachActionLog when the model " + "declares one matching ModelLevelActionLogAttachable", + "[action_log][holder]") { + static_assert(morph::model::detail::ModelLevelActionLogAttachable); + static_assert(!morph::model::detail::ModelLevelActionLogAttachable); + + auto holder = morph::model::detail::ModelFactory::create(); + auto log = std::make_shared(); + holder->attachActionLog(log, "board-7"); + + // The holder's own auto-append state (recordIfAttached/hasActionLog) + // still works exactly as before -- this hook adds a second, independent + // forward, it doesn't replace the holder's own bookkeeping. + REQUIRE(holder->hasActionLog()); + + auto& typed = static_cast&>(*holder); + REQUIRE(typed.model.callCount == 1); + REQUIRE(typed.model.log == log); + REQUIRE(typed.model.contextKey == "board-7"); +} + +TEST_CASE("IModelHolder::onActionLogAttached: the no-op default runs when a holder wraps a model with no " + "model-level attachActionLog", + "[action_log][holder]") { + // ALModel doesn't declare attachActionLog, so ModelHolder:: + // onActionLogAttached resolves its `if constexpr` false and never calls + // into the model -- attachActionLog must still succeed and populate the + // holder's own state exactly as it did before this hook existed. + auto holder = morph::model::detail::ModelFactory::create(); + auto log = std::make_shared(); + REQUIRE_NOTHROW(holder->attachActionLog(log, "acct-noop")); + REQUIRE(holder->hasActionLog()); +} + TEST_CASE("IModelHolder: recordIfAttached captures the active session principal", "[action_log][holder]") { auto holder = morph::model::detail::ModelFactory::create(); auto log = std::make_shared();