core+journal: a journal sink that refuses a success no longer makes the framework report — and record — a committed write as rejected (fixes #796)

docs/spec/core/bridge.md

@@ -194,14 +194,20 @@ throw at runtime means `LocalBackend` was used in a build that promised never
 to — a configuration error, not a normal failure mode.
 
 Otherwise (the default, non-`MORPH_CLIENT_ONLY` build), `Model::execute(*action)`
-itself is wrapped in a `try`/`catch (const std::exception&)`: on success it
-records a journal `LogEntry` with `outcome = Outcome::Succeeded` for loggable
-actions; on a throw it records `outcome = Outcome::Failed` (`error =
-exc.what()`, `result` empty) for the same actions and rethrows unchanged, so
-the exception still resolves the `Completion` through `onError` exactly as
-before — the journal entry is a side effect of the attempt, not a change to
-error propagation. Mirrors `ActionDispatcher::registerAction`'s runner
-(`registry.md`) for remote topologies. See [journal.md,
+itself — and nothing else — is wrapped in a `try`/`catch (const std::exception&)`:
+on a throw it records `outcome = Outcome::Failed` (`error = exc.what()`,
+`result` empty) for loggable actions and rethrows unchanged, so the exception
+still resolves the `Completion` through `onError` exactly as before — the
+journal entry is a side effect of the attempt, not a change to error
+propagation. Serialising the result and recording `outcome = Outcome::Succeeded`
+run after that `try`, because by then the mutation has committed: a sink whose
+`append` throws, or a result that will not serialise, resolves the `Completion`
+through `onError` with `morph::model::ActionRecordingError` rather than being
+reported — and journaled — as the model refusing the action. Mirrors
+`ActionDispatcher::registerAction`'s runner (`registry.md`) for remote
+topologies. See [journal.md, "A refused recording is not an execution
+failure"](../journal/journal.md#a-refused-recording-is-not-an-execution-failure)
+and [journal.md,
 "Outcome"](../journal/journal.md#logentry--one-recorded-action-execution) for
 the full field/replay semantics.
 
@@ -1105,7 +1111,7 @@ make teardown order-independent.)
 | `registerHandler(binding)` | `void registerHandler(const shared_ptr<HandlerBinding>&)` | Pre-built binding. Same async-preferring behavior. |
 | `switchBackend` | `void switchBackend(unique_ptr<IBackend>)` / `void switchBackend(shared_ptr<IBackend>)` | Pushes the current default session onto the new backend via `setSession` before staging. Stages all re-registrations through `bindModel` on the new backend, commits (publishes new ids + swaps) only if all succeed, else rolls back and rethrows leaving old backend + `currentId`s intact. Atomic exactly when the new backend answers `kCallerMayBlock`; a `kCallerMustNotBlock` backend's binds are deferred and the switch is not all-or-nothing (see above). Cancels old backend's pending ops with `BackendChangedError`. Holds both `_mtx` and `_attachMtx` for its staging and commit, and resolves `whenBound()` waiters after releasing them. The `unique_ptr` overload is a template on the concrete backend type and delegates to the `shared_ptr` one — see below. |
 | `deregisterHandler` | `void deregisterHandler(const shared_ptr<HandlerBinding>&)` | Deregisters from active backend (if bound), resets `currentId` to 0, removes from tracking. |
-| `executeVia<Model, Action>` | `Completion<R> executeVia(const shared_ptr<HandlerBinding>&, Action, IExecutor*)` | Lock-free dispatch. Attaches default session. On `LocalBackend`, rejects an action whose `ActionValidator::ready` returns `false` with `morph::model::ValidationError` via `onError`, before `Model::execute` runs. Records a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged). Dispatches through `IBackend::executeInto`, handing the backend a `detail::BridgeSink<R>` that is simultaneously the caller's typed completion state and the backend's settle sink — one allocation where an erased-completion forwarding block costs six. Value-forwarding into the typed `Completion` is `try`/`catch`-guarded — a throwing result move/copy resolves the completion via `onError` instead of hanging or terminating. The bridge-touching side effects (`onResult`, `hasSubscribers()`/`publishResult`, the `pendingCalls()` decrement, and the execute-deadline disarm) are gated on the bridge's `CallbackToken`, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. Arms the client-side execute deadline when one is installed (see `setExecuteDeadline`); the fast-fail "handler not bound" path returns before that and arms nothing. |
+| `executeVia<Model, Action>` | `Completion<R> executeVia(const shared_ptr<HandlerBinding>&, Action, IExecutor*)` | Lock-free dispatch. Attaches default session. On `LocalBackend`, rejects an action whose `ActionValidator::ready` returns `false` with `morph::model::ValidationError` via `onError`, before `Model::execute` runs. Records a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged); a failure to serialise the result or to append the success entry happens after the mutation committed and rejects the completion with `morph::model::ActionRecordingError` instead of recording `Outcome::Failed`. Dispatches through `IBackend::executeInto`, handing the backend a `detail::BridgeSink<R>` that is simultaneously the caller's typed completion state and the backend's settle sink — one allocation where an erased-completion forwarding block costs six. Value-forwarding into the typed `Completion` is `try`/`catch`-guarded — a throwing result move/copy resolves the completion via `onError` instead of hanging or terminating. The bridge-touching side effects (`onResult`, `hasSubscribers()`/`publishResult`, the `pendingCalls()` decrement, and the execute-deadline disarm) are gated on the bridge's `CallbackToken`, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. Arms the client-side execute deadline when one is installed (see `setExecuteDeadline`); the fast-fail "handler not bound" path returns before that and arms nothing. |
 | `setDefaultSession` | `void setDefaultSession(session::Context)` | Installs default session context; also pushes it to the active backend via `IBackend::setSession` so control envelopes (register/attach/assign/deregister) carry it too, not only `execute`. |
 | `defaultSession` | `session::Context defaultSession() const` | Returns snapshot of default session. |
 | `setExecuteDeadline` | `void setExecuteDeadline(std::chrono::milliseconds)` | Opt-in client-side execute deadline; `0` (the default) disables it. Lazily creates the backing `TimeoutScheduler` thread on first enable. |

docs/spec/core/registry.md

@@ -55,6 +55,12 @@ provides:
   runner (`ActionDispatcher::registerAction`), and the local `Bridge::executeVia`
   path — the last two throw `ValidationError` on a `false` result instead of
   running `Model::execute`.
+- **`ActionRecordingError`** — thrown when an action executed and its mutation
+  committed, but serialising the result or appending the journal entry failed.
+  Carries `cause()` (the underlying message) and `result()` (the committed
+  action's result JSON, or `""` when serialising it is what failed). Derives
+  from `std::runtime_error`, so existing `catch (const std::exception&)` paths
+  are unaffected.
 - **Logging policy** — `ActionLogPolicy<A>` and `Loggable` that control whether
   an action's executions are recorded and how duplicates are coalesced.
 - **Type-erased holders** — `IModelHolder` / `ModelHolder<M>` that own a model
@@ -600,12 +606,18 @@ class ActionDispatcher {
   enforcement and before the validator check, so the validator sees the
   authoritative computed value), enforces `ActionValidator<Action>::ready(action)` (throwing `ValidationError`
   on `false`, before `Model::execute` runs), then calls `Model::execute(action)`
-  inside a `try`/`catch (const std::exception&)`: on success it serialises the
-  result and records a `LogEntry` with `outcome = Outcome::Succeeded` (when
-  loggable and a log is attached); on a throw it records `outcome =
-  Outcome::Failed` (`error = exc.what()`, `result` empty) for the same actions
-  and rethrows unchanged, so callers see the same exception as before — the
-  journal entry is a side effect, not a change to error propagation. Mirrors
+  inside a `try`/`catch (const std::exception&)`: on a throw it records
+  `outcome = Outcome::Failed` (`error = exc.what()`, `result` empty) when the
+  action is loggable and a log is attached, and rethrows unchanged, so callers
+  see the same exception as before — the journal entry is a side effect, not a
+  change to error propagation. `Model::execute` is the **only** call inside that
+  `try`: serialising the result and recording `outcome = Outcome::Succeeded` run
+  after it, outside, because by then the mutation has committed and a throw from
+  either is not an execution failure. Both surface as
+  `morph::model::ActionRecordingError` instead — see [journal.md, "A refused
+  recording is not an execution
+  failure"](../journal/journal.md#a-refused-recording-is-not-an-execution-failure).
+  Mirrors
   `Bridge::executeVia`'s `localOp` (`bridge.md`) for `LocalBackend`. See
   [journal.md, "Outcome"](../journal/journal.md#logentry--one-recorded-action-execution)
   for the full field/replay semantics. Every recorded entry — success or

docs/spec/journal/journal.md

@@ -27,6 +27,7 @@ by `contextKey`; see [Attaching a log to remote instances](#attaching-a-log-to-r
 ## Contents
 
 - [LogEntry — one recorded action execution](#logentry--one-recorded-action-execution)
+  - [A refused recording is not an execution failure](#a-refused-recording-is-not-an-execution-failure)
 - [Serialization](#serialization)
   - [Why the codec is a separate header](#why-the-codec-is-a-separate-header)
 - [Line-format version (`v`)](#line-format-version-v)
@@ -84,6 +85,49 @@ the `catch` records a `Failed` entry (`error = exc.what()`, `result` empty)
 and rethrows unchanged, so the caller's error handling is unaffected — only
 the journal gains an entry it previously lacked.
 
+### A refused recording is not an execution failure
+
+`Outcome::Failed` means the model rejected the action, and nothing else. Only
+`Model::execute` can produce it, so it is the only call inside the `try` that
+records one.
+
+Two steps run after it and before the caller is answered: serialising the
+result (`ActionTraits<Action>::resultToJson`, which raises
+`model::detail::ParseError` on a write error) and appending the `LogEntry`. Both
+can throw, and by the time either does the model's mutation has already
+committed — an `IActionLog` that could not reach its backend is *required* to
+throw, since `append`/`flush` return `void` and that is the only channel the
+interface gives it (`FileActionLog` throws from eighteen sites; a full disk or a
+revoked permission reaches them in production).
+
+Inside the execution `try` they would give three wrong answers at once: a caller
+told a durable write was rejected and free to retry it, an `Outcome::Failed`
+entry in the audit trail for a mutation that committed, and that entry's `error`
+carrying the *sink's* message, permanently blaming the action for an
+infrastructure fault. An audit log that is wrong about which actions succeeded is
+worse than one missing entries, because nothing downstream can tell the two apart.
+
+So both steps sit outside that `try`, and a throw from either surfaces as
+`morph::model::ActionRecordingError` — a `std::runtime_error` subclass whose
+`what()` is `"action executed but was not recorded: <cause>"`, with `cause()`
+returning the underlying message and `result()` the committed action's result
+JSON. The caller still learns the recording failed; what changed is what it is
+told, which is now true: the write happened, the record of it did not. Existing
+`catch (const std::exception&)` handling is unaffected — `RemoteServer` still
+turns it into an `err` reply, `LocalBackend` still rejects the `Completion`
+through `onError` — and a caller that wants to distinguish a
+committed-but-unrecorded action from a rejected one catches the type.
+
+When the throw came from `resultToJson`, **no entry is written at all**. A
+`Succeeded` entry carries the result by definition (see `LogEntry::result`
+above), and there is none to carry; the caller is told, which is the only
+channel left. A committed mutation with no entry is a gap, but a smaller one
+than an entry asserting it failed.
+
+`OutboxRelay` is unaffected by any of this: it calls `sink->append()` and
+`sink->flush()` directly, and depends on their throwing — an outbox row is marked
+relayed only after the sink returned normally.
+
 ## Serialization
 
 ### Why the codec is a separate header
@@ -1307,6 +1351,13 @@ These hold for every sink and are relied on by `replay()`/`undoLast()`:
   success, so replaying `payload` re-derives an equivalent `result` for a
   deterministic model. A `Failed` entry has no `result` to derive — see
   `replay()`, next.
+- **A `Failed` entry means the model rejected the action.** It never means the
+  framework could not record a success. Serialising the result and appending the
+  entry run after the mutation has committed, outside the `try` that records
+  `Failed`; a throw from either is reported to the caller as
+  `morph::model::ActionRecordingError` and files no entry. See [A refused
+  recording is not an execution
+  failure](#a-refused-recording-is-not-an-execution-failure).
 - **`replay()`/`undoLast()` skip `Failed` entries.** A failed attempt never
   mutated model state, so there is nothing to reconstruct from it — and
   re-dispatching it would likely throw the same exception again, aborting

include/morph/core/bridge.hpp

@@ -2108,35 +2108,64 @@ class Bridge {
             // Local mode has no client/server split, so this is the same execution
             // site `ActionDispatcher::registerAction`'s runner is for remote modes
             // (registry.hpp) — see that overload's doc comment for the full story,
-            // including why both the success and failure paths below record a
-            // journal entry (a rejected/throwing execute must not leave the audit
-            // trail silent) and why the exception is rethrown unchanged either way.
-            try {
-                auto result = std::make_shared<R>(model.execute(actionRef));
-                if constexpr (::morph::model::detail::actionLoggable<Action>() == ::morph::model::Loggable::Yes) {
-                    if (holder.hasActionLog()) {
-                        // entityKey/principal/timestampMs are filled in by recordIfAttached.
-                        ::morph::model::detail::recordActionSuccess(
-                            holder, std::string{::morph::model::ModelTraits<Model>::typeId()},
-                            std::string{::morph::model::ActionTraits<Action>::typeId()},
-                            ::morph::model::ActionTraits<Action>::toJson(actionRef),
-                            ::morph::model::detail::actionPayloadSchema<Action>(),
-                            ::morph::model::ActionTraits<Action>::resultToJson(*result));
+            // including why a rejected/throwing execute must not leave the audit
+            // trail silent, and why `Model::execute` is the only call inside the
+            // try that records Outcome::Failed.
+// MSVC's C4702 fires on the `return` below for any action whose handler never
+// returns -- a test double whose body is a bare `throw`, for instance. The
+// warning is correct for that instantiation and wrong as a verdict on this
+// statement, which every other instantiation reaches. It is suppressed here
+// rather than in each translation unit that instantiates such a handler,
+// because the set of those is open-ended: eleven test files already qualify.
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4702)
+#endif
+            auto result = [&] {
+                try {
+                    return std::make_shared<R>(model.execute(actionRef));
+                } catch (const std::exception& exc [[maybe_unused]]) {
+                    if constexpr (::morph::model::detail::actionLoggable<Action>() == ::morph::model::Loggable::Yes) {
+                        if (holder.hasActionLog()) {
+                            ::morph::model::detail::recordActionFailure(
+                                holder, std::string{::morph::model::ModelTraits<Model>::typeId()},
+                                std::string{::morph::model::ActionTraits<Action>::typeId()},
+                                ::morph::model::ActionTraits<Action>::toJson(actionRef),
+                                ::morph::model::detail::actionPayloadSchema<Action>(), exc.what());
+                        }
                     }
+                    throw;
                 }
-                return result;
-            } catch (const std::exception& exc [[maybe_unused]]) {
-                if constexpr (::morph::model::detail::actionLoggable<Action>() == ::morph::model::Loggable::Yes) {
-                    if (holder.hasActionLog()) {
-                        ::morph::model::detail::recordActionFailure(
+            }();
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+            // Past this point the model's mutation has committed, so neither
+            // serialising the result nor appending the entry may be reported as
+            // an execution failure: both throw (ParseError; a sink that could
+            // not reach its backend), and inside the try above that throw would
+            // reject this call's Completion as if the model had refused the
+            // action and file an Outcome::Failed entry blaming the action for an
+            // infrastructure fault. ActionRecordingError says what is true
+            // instead -- the action ran, the recording of it did not -- and
+            // carries the result JSON the audit trail never received.
+            if constexpr (::morph::model::detail::actionLoggable<Action>() == ::morph::model::Loggable::Yes) {
+                if (holder.hasActionLog()) {
+                    std::string resultJson;
+                    try {
+                        resultJson = ::morph::model::ActionTraits<Action>::resultToJson(*result);
+                        // entityKey/principal/timestampMs are filled in by recordIfAttached.
+                        ::morph::model::detail::recordActionSuccess(
                             holder, std::string{::morph::model::ModelTraits<Model>::typeId()},
                             std::string{::morph::model::ActionTraits<Action>::typeId()},
                             ::morph::model::ActionTraits<Action>::toJson(actionRef),
-                            ::morph::model::detail::actionPayloadSchema<Action>(), exc.what());
+                            ::morph::model::detail::actionPayloadSchema<Action>(), resultJson);
+                    } catch (const std::exception& exc) {
+                        throw ::morph::model::ActionRecordingError{std::move(resultJson), exc.what()};
                     }
                 }
-                throw;
             }
+            return result;
 #endif
         };
         {