fix(gateway): forward remote-entity writes; harden DTO request/response contract

Aggregation forwarding only proxied GET and SSE: the typed body, alternates,
delete-alternates, multipart, and binary wrappers never installed the
ForwardResponseScope, so writes and uploads to a remote entity returned an
empty no-op response instead of the peer's. Install the scope uniformly in
every wrapper (matching the documented "around every typed handler invocation"
invariant) and add forwarding regression tests for PUT, POST-alternates,
DELETE-alternates, and multipart routes.

Tighten the DTO contract:

- JsonReader treats an explicit JSON null as a value for free-form json and
  optional<json> members, so PUT data/configurations with {"data": null} no
  longer returns 400; null on other member types is still treated as absent.
- ScriptControlRequest.action uses plain field() instead of field_enum so
  plugin script backends may accept actions beyond the built-in
  stop/forced_termination; the provider validates the value.
- Restore the FaultListItem.severity_label response enum (now including
  UNKNOWN, which fault_to_json emits for out-of-range severities) and pin
  fault_to_json output to the FaultListItem schema with a round-trip test, so
  the verbatim fault-list wire cannot drift from its published schema.
- JsonWriter's value encoder ends in a static_assert like the reader and schema
  visitors, so an unsupported field type fails to compile instead of silently
  taking the scalar branch.
- Drop a no-op multipart parts initialization.

Document the synchronous operation service-call failure error-envelope
normalization and the fan-out re-serialization semantics in the changelog.

Author: bburda

src/ros2_medkit_gateway/CHANGELOG.rst

@@ -48,13 +48,24 @@ Unreleased
   covered by this PR - its handlers continue to run on the pre-typed
   HandlerContext surface and will be migrated separately
   (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
+* Synchronous operation-execution service-call failures
+  (``POST /api/v1/{entity-path}/operations/{id}/executions`` when the underlying
+  ROS 2 service call fails) now return the standard SOVD ``GenericError`` envelope
+  (``{"error_code": "vendor-error", "vendor_code":
+  "x-medkit-ros2-service-unavailable", "message": "Service call failed", ...}``,
+  HTTP status 500 unchanged) instead of the previous bespoke nested
+  ``{"error": {"code", "message", "details"}}`` object. This aligns the one
+  remaining non-standard error path with every other gateway error; clients that
+  parsed ``error.code`` / ``error.details`` for this specific failure must read
+  ``vendor_code`` / ``parameters`` instead
+  (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
 * ``ros2_medkit_msgs/srv/ClearFault`` request gains a ``bool skip_correlation_auto_clear`` field (see the per-entity fault scope entry below for the in-tree motivation). Adding a request field changes the service type hash, so out-of-tree callers that invoke the service directly (for example ``ros2 service call /fault_manager/clear_fault ros2_medkit_msgs/srv/ClearFault ...`` as documented in the ``ros2_medkit_fault_manager`` README) must rebuild against the new ``ros2_medkit_msgs`` to keep talking to ``fault_manager``. The in-tree gateway client and server are updated together (`#395 <https://github.com/selfpatch/ros2_medkit/issues/395>`_)
 * Per-entity fault routes are now correctly scoped to the entity's hosted apps. ``GET /api/v1/{entity-path}/faults/{fault_code}``, ``DELETE /api/v1/{entity-path}/faults/{fault_code}``, ``GET /api/v1/{entity-path}/faults``, and ``DELETE /api/v1/{entity-path}/faults`` previously fell back to a prefix match against the entity's ``namespace_path``; when that was empty (host-derived / synthetic components, manifest components without a ``namespace`` field, Areas, Functions, and Apps with a wildcard ``ros_binding.namespace_pattern``) the scope filter was silently disabled and the routes exposed - and on ``DELETE``, cleared - faults reported by apps that belonged to entirely different entities. All four handlers now resolve the addressed entity to its hosted-app FQN set (via the new ``HandlerContext::resolve_entity_source_fqns`` helper) and apply a strict all-sources scope check: a fault counts as in scope only when **every** entry in its ``reporting_sources`` is owned by the entity (exact FQN match, or strict path-child via ``<fqn>/...``). Per-fault routes return ``404 Resource Not Found`` for any fault that fails the check; collection routes return an empty ``items`` array. The underlying ``GetFault.srv`` contract is unchanged; ``ClearFault.srv`` gains a new ``skip_correlation_auto_clear`` request flag so per-entity DELETE can opt out of cascade-clearing correlated symptom fault codes that may live in other entities. Per-entity collection responses no longer include the global ``muted_count`` / ``cluster_count`` / ``muted_faults`` / ``clusters`` correlation metadata; those remain on the global ``GET /api/v1/faults`` route. Behavior changes visible to clients: (a) faults reported by apps outside the addressed entity are no longer returned or cleared via that entity's route, (b) **mixed-source** faults that include at least one out-of-entity reporter are likewise rejected with ``404`` on per-fault routes and excluded from per-entity collection responses (use the global ``GET /api/v1/faults`` to see them), (c) per-entity DELETE no longer cascade-clears correlated symptoms outside the entity (`#395 <https://github.com/selfpatch/ros2_medkit/issues/395>`_)
 * ``GET /api/v1/updates/{id}/status`` no longer returns ``404`` for a registered-but-idle package; ``POST /api/v1/updates`` now seeds a ``pending`` status, so the endpoint returns ``200 {"status": "pending"}`` immediately after registration. ``404`` is reserved for packages that are not registered. Clients that used ``404`` as a signal for "registered but nothing started yet" must adapt (`#378 <https://github.com/selfpatch/ros2_medkit/issues/378>`_)
 
 **Features:**
 
-* Typed ``fan_out_collection<T>`` aggregating helper replaces raw-JSON ``merge_peer_items`` on the typed collection routes (data, operations, config, logs). Peer items are decoded via ``dto::JsonReader<T>``; items that fail validation are removed from the merged ``items`` array, recorded in ``x-medkit.peer_dropped_items`` with the JsonReader error plus a best-effort ``source_id``, and logged at ``WARN``. Previously, malformed peer items silently disappeared into the merged response; fleet operators can now detect inter-gateway schema drift directly on the wire (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
+* Typed ``fan_out_collection<T>`` aggregating helper replaces raw-JSON ``merge_peer_items`` on the typed collection routes (data, operations, config, logs). Peer items are decoded via ``dto::JsonReader<T>``; items that fail validation are removed from the merged ``items`` array, recorded in ``x-medkit.peer_dropped_items`` with the JsonReader error plus a best-effort ``source_id``, and logged at ``WARN``. Items that parse successfully are re-serialized through the local ``dto::JsonWriter<T>``, so any peer-supplied fields outside the local DTO schema are dropped from the merged response (the previous raw passthrough preserved unknown peer fields verbatim). Previously, malformed peer items silently disappeared into the merged response; fleet operators can now detect inter-gateway schema drift directly on the wire (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
 * ``Collection<T, XMedkitT>`` is now a 2-parameter template. Domain list endpoints (faults, config, data, logs) reference their richer per-domain collection x-medkit struct (``FaultListXMedkit``, ``ConfigListXMedkit``, ``DataListXMedkit``, ``LogListXMedkit``) directly in the published schema instead of the generic ``XMedkitCollection``, so generated clients see aggregation counts, peer provenance, and ``peer_dropped_items`` from the schema (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
 * New ``opaque_object("key", &T::field)`` DTO field descriptor in ``dto/contract.hpp``. Binds a ``nlohmann::json`` member as a typed "any JSON object" field: ``JsonWriter`` emits it verbatim, ``JsonReader`` rejects scalars / arrays / null, ``SchemaWriter`` emits ``{type: object, additionalProperties: true, x-medkit-opaque: true}``. Used for fields whose runtime shape is decided by an upstream component the gateway cannot introspect (live ROS message payloads, plugin-defined fault envelopes, action results) (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
 * ``GET /api/v1/faults/stream`` event payloads now carry an optional ``x-medkit`` SOVD payload-extension object with ``entity_type`` and ``entity_id`` fields. When the gateway can resolve the fault's first reporting source back to a SOVD entity (via the manifest-mode linking index, or a runtime-mode last-segment match against an existing App), consumers can hit ``/{entity_type}/{entity_id}/bulk-data/rosbags/{fault_code}`` directly instead of HEAD-probing every entity. Resolution is snapshotted at event arrival, so a discovery refresh between enqueue and stream-out cannot retroactively change the entity reported to consumers. The ``x-medkit`` object is omitted entirely when no entity can be resolved, so existing SSE consumers ignore the addition (`#380 <https://github.com/selfpatch/ros2_medkit/issues/380>`_)

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/enums.hpp

@@ -22,8 +22,10 @@ namespace dto {
 /// Entity types (used in entity detail responses).
 inline constexpr std::string_view kEntityTypeValues[] = {"area", "component", "app", "function"};
 
-/// Fault severity label (fault_list_item_schema / fault_detail_schema).
-inline constexpr std::string_view kFaultSeverityLabelValues[] = {"INFO", "WARN", "ERROR", "CRITICAL"};
+/// Fault severity label (FaultListItem.severity_label). Must include "UNKNOWN":
+/// fault_to_json emits it for any severity outside the four known levels
+/// (fault_msg_conversions.cpp default branch).
+inline constexpr std::string_view kFaultSeverityLabelValues[] = {"INFO", "WARN", "ERROR", "CRITICAL", "UNKNOWN"};
 
 /// Fault aggregated status (fault_detail_schema - status.aggregatedStatus).
 inline constexpr std::string_view kFaultAggregatedStatusValues[] = {"active", "passive", "cleared"};
@@ -53,8 +55,5 @@ inline constexpr std::string_view kLogSeverityFilterValues[] = {"debug", "info",
 /// Execution control capability (execution_update_request_schema).
 inline constexpr std::string_view kExecutionCapabilityValues[] = {"stop", "execute", "freeze", "reset"};
 
-/// Script control action (script_control_request_schema).
-inline constexpr std::string_view kScriptControlActionValues[] = {"stop", "forced_termination"};
-
 }  // namespace dto
 }  // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/faults.hpp

@@ -62,7 +62,7 @@ inline constexpr auto dto_fields<FaultListItem> = std::make_tuple(
     field("description", &FaultListItem::description), field("first_occurred", &FaultListItem::first_occurred),
     field("last_occurred", &FaultListItem::last_occurred), field("occurrence_count", &FaultListItem::occurrence_count),
     field("status", &FaultListItem::status), field("reporting_sources", &FaultListItem::reporting_sources),
-    field("severity_label", &FaultListItem::severity_label));
+    field_enum("severity_label", &FaultListItem::severity_label, kFaultSeverityLabelValues));
 
 template <>
 inline constexpr std::string_view dto_name<FaultListItem> = "FaultListItem";

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/json_reader.hpp

@@ -133,7 +133,20 @@ struct JsonReader {
         auto & member = obj.*(f.ptr);
         using MemberT = std::decay_t<decltype(member)>;
         const auto it = j.find(key);
-        if (it == j.end() || it->is_null()) {
+        // A present-but-null JSON value is a legitimate value for free-form json
+        // members (e.g. PUT .../data/{id} or .../configurations/{id} with
+        // {"data": null} to set a parameter to null). For every other member type
+        // null cannot be coerced, so it is treated like an absent field.
+        const bool present = (it != j.end());
+        bool treat_as_absent = !present;
+        if (present && it->is_null()) {
+          if constexpr (is_optional_v<MemberT>) {
+            treat_as_absent = !std::is_same_v<typename MemberT::value_type, nlohmann::json>;
+          } else {
+            treat_as_absent = !std::is_same_v<MemberT, nlohmann::json>;
+          }
+        }
+        if (treat_as_absent) {
           if (f.presence == Presence::kRequired) {
             errs.push_back({key, "missing required field"});
           }

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/json_writer.hpp

@@ -44,9 +44,15 @@ nlohmann::json encode_value(const U & v) {
           return encode_value(alt);
         },
         v);
-  } else {
+  } else if constexpr (std::is_same_v<U, std::string> || std::is_same_v<U, bool> || std::is_integral_v<U> ||
+                       std::is_floating_point_v<U> || std::is_same_v<U, nlohmann::json>) {
     // string / bool / integral / floating / nlohmann::json passthrough
     return nlohmann::json(v);
+  } else {
+    // Mirror decode_value / schema_of: fail loud if a field type leaks here that
+    // is neither a DTO, container, variant, nor a supported scalar (e.g. a DTO
+    // sentinel used in a TU that never saw its dto_fields specialization).
+    static_assert(sizeof(U) == 0, "encode_value: unsupported field type");
   }
 }
 

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/scripts.hpp

@@ -23,7 +23,6 @@
 
 #include "ros2_medkit_gateway/dto/contract.hpp"
 #include "ros2_medkit_gateway/dto/entities.hpp"
-#include "ros2_medkit_gateway/dto/enums.hpp"
 
 namespace ros2_medkit_gateway {
 namespace dto {
@@ -193,20 +192,23 @@ inline constexpr std::string_view dto_name<ScriptUploadResponse> = "ScriptUpload
 //
 // Wire shape (from script_control_request_schema() + handle_control_execution()):
 //   action - control action to apply (required)
-//            enum: "stop" | "forced_termination"
-//
-// Uses field_enum: the control handler validates only that "action" is present
-// and non-empty (ERR_INVALID_REQUEST for missing). It does NOT perform bespoke
-// value-range validation with ERR_INVALID_PARAMETER; parse_body provides the
-// enum check instead.
+//            built-in backend: "stop" | "forced_termination"
+//
+// Uses plain field() (NOT field_enum): control_execution forwards `action`
+// verbatim to the ScriptProvider, which may be a plugin backend supporting
+// actions beyond the built-in stop/forced_termination (e.g. pause/resume).
+// Constraining the value at parse time would block those plugins at the
+// gateway. The handler validates presence (ERR_INVALID_REQUEST when missing);
+// the provider validates the value. (Same reasoning as
+// ExecutionUpdateRequest.capability.)
 // =============================================================================
 struct ScriptControlRequest {
-  std::string action;  // enum: stop | forced_termination
+  std::string action;  // built-in backend: stop | forced_termination; plugins may extend
 };
 
 template <>
 inline constexpr auto dto_fields<ScriptControlRequest> =
-    std::make_tuple(field_enum("action", &ScriptControlRequest::action, kScriptControlActionValues));
+    std::make_tuple(field("action", &ScriptControlRequest::action));
 
 template <>
 inline constexpr std::string_view dto_name<ScriptControlRequest> = "ScriptControlRequest";

src/ros2_medkit_gateway/src/core/openapi/route_registry.cpp

@@ -267,6 +267,9 @@ RouteRegistry::binary_download(const std::string & openapi_path,
                                std::function<http::Result<http::BinaryResponse>(http::TypedRequest)> handler) {
   auto renderer = std::make_shared<ErrorRenderer>(ErrorRenderer::kSovdGenericError);
   HandlerFn fn = [handler = std::move(handler), renderer](const httplib::Request & req, httplib::Response & res) {
+    // Forwarding scope: entity-scoped binary downloads (bulk-data, scripts) on a
+    // remote peer must proxy through validate_entity_for_route (see sse / wrap_body_less).
+    http::detail::ForwardResponseScope forward_scope(&res);
     http::TypedRequest typed_req(req);
     auto outcome = handler(typed_req);
     if (!outcome.has_value()) {
@@ -302,6 +305,9 @@ RouteEntry & RouteRegistry::static_asset(const std::string & openapi_path,
                                          std::function<http::Result<http::StaticAsset>(http::TypedRequest)> handler) {
   auto renderer = std::make_shared<ErrorRenderer>(ErrorRenderer::kSovdGenericError);
   HandlerFn fn = [handler = std::move(handler), renderer](const httplib::Request & req, httplib::Response & res) {
+    // Forwarding scope kept uniform across wrappers (static assets are not
+    // entity-scoped, so this never forwards; see the comment at the top of this file).
+    http::detail::ForwardResponseScope forward_scope(&res);
     http::TypedRequest typed_req(req);
     auto outcome = handler(typed_req);
     if (!outcome.has_value()) {
@@ -326,6 +332,8 @@ RouteEntry & RouteRegistry::docs_endpoint(const std::string & openapi_path,
                                           std::function<http::Result<nlohmann::json>(http::TypedRequest)> handler) {
   auto renderer = std::make_shared<ErrorRenderer>(ErrorRenderer::kSovdGenericError);
   HandlerFn fn = [handler = std::move(handler), renderer](const httplib::Request & req, httplib::Response & res) {
+    // Forwarding scope kept uniform across wrappers (see the comment at the top of this file).
+    http::detail::ForwardResponseScope forward_scope(&res);
     http::TypedRequest typed_req(req);
     auto outcome = handler(typed_req);
     if (!outcome.has_value()) {

src/ros2_medkit_gateway/src/openapi/route_registry.hpp

@@ -556,6 +556,11 @@ HandlerFn RouteRegistry::wrap_with_body(std::function<http::Result<TResponse>(ht
                                         std::shared_ptr<ErrorRenderer> renderer) {
   return [handler = std::move(handler), renderer = std::move(renderer)](const httplib::Request & req,
                                                                         httplib::Response & res) {
+    // Forwarding scope: lets the typed validate_entity_for_route stream a
+    // proxied response when the entity is owned by a remote peer (see
+    // wrap_body_less). Without it, remote-entity writes return Forwarded with
+    // no sink and the client gets an empty body instead of the peer response.
+    http::detail::ForwardResponseScope forward_scope(&res);
     auto body = detail::parse_request_body<TBody>(req);
     if (!body.has_value()) {
       write_typed_error(res, body.error(), renderer);
@@ -577,6 +582,8 @@ HandlerFn RouteRegistry::wrap_with_body_attachments(
     std::shared_ptr<ErrorRenderer> renderer) {
   return [handler = std::move(handler), renderer = std::move(renderer)](const httplib::Request & req,
                                                                         httplib::Response & res) {
+    // Forwarding scope for remote-peer entities (see wrap_body_less / wrap_with_body).
+    http::detail::ForwardResponseScope forward_scope(&res);
     auto body = detail::parse_request_body<TBody>(req);
     if (!body.has_value()) {
       write_typed_error(res, body.error(), renderer);
@@ -601,6 +608,8 @@ HandlerFn RouteRegistry::wrap_post_alternates(
     std::shared_ptr<ErrorRenderer> renderer) {
   return [handler = std::move(handler), renderer = std::move(renderer)](const httplib::Request & req,
                                                                         httplib::Response & res) {
+    // Forwarding scope for remote-peer entities (see wrap_body_less / wrap_with_body).
+    http::detail::ForwardResponseScope forward_scope(&res);
     auto body = detail::parse_request_body<TBody>(req);
     if (!body.has_value()) {
       write_typed_error(res, body.error(), renderer);
@@ -629,6 +638,8 @@ HandlerFn RouteRegistry::wrap_post_alternates_with_attachments(
     std::shared_ptr<ErrorRenderer> renderer) {
   return [handler = std::move(handler), renderer = std::move(renderer)](const httplib::Request & req,
                                                                         httplib::Response & res) {
+    // Forwarding scope for remote-peer entities (see wrap_body_less / wrap_with_body).
+    http::detail::ForwardResponseScope forward_scope(&res);
     auto body = detail::parse_request_body<TBody>(req);
     if (!body.has_value()) {
       write_typed_error(res, body.error(), renderer);
@@ -659,6 +670,8 @@ RouteRegistry::wrap_del_alternates(std::function<http::Result<std::variant<TAlt.
                                    std::shared_ptr<ErrorRenderer> renderer) {
   return [handler = std::move(handler), renderer = std::move(renderer)](const httplib::Request & req,
                                                                         httplib::Response & res) {
+    // Forwarding scope for remote-peer entities (see wrap_body_less / wrap_with_body).
+    http::detail::ForwardResponseScope forward_scope(&res);
     http::TypedRequest typed_req(req);
     auto outcome = handler(typed_req);
     if (outcome.has_value()) {
@@ -980,8 +993,10 @@ RouteEntry & RouteRegistry::multipart_upload(
                 "multipart_upload<T>: T must be a DTO (or NoContent)");
   auto renderer = std::make_shared<ErrorRenderer>(ErrorRenderer::kSovdGenericError);
   HandlerFn fn = [handler = std::move(handler), renderer](const httplib::Request & req, httplib::Response & res) {
+    // Forwarding scope for remote-peer entities (see wrap_body_less / wrap_with_body).
+    http::detail::ForwardResponseScope forward_scope(&res);
     http::MultipartBody body;
-    body.parts = req.files.empty() ? httplib::MultipartFormDataItems{} : httplib::MultipartFormDataItems{};
+    // body.parts default-constructs empty; the loop below populates it from req.files.
     // cpp-httplib exposes parsed multipart entries via `req.files`; surface
     // them through MultipartBody.parts as MultipartFormData entries.
     for (const auto & [name, file] : req.files) {