fix(openapi): tighten query-parameter contract and harden its regression gate

Constrain query DTOs to the scalar shapes the parser actually supports and gate
both the schema writer and the request reader on one trait, so the OpenAPI
parameters and the parser can no longer advertise different types, an unenforced
`required` flag, or a boolean the parser reads differently than the schema.

- dto/query.hpp: is_query_scalar_v (string/bool) is now the single source of
  truth enforced by both QueryParamWriter and assign_query_field; static_assert
  that no query field is required (the parser never enforces presence); parse
  booleans as true/1 case-insensitively to match the advertised type: boolean.
  Scope the no-drift guarantee to parameter names/presence - value validation
  (enum membership, format) stays the handler's responsibility.
- dto/faults.hpp: add FaultEntityListQuery (status only) for the per-entity
  GET /{entity}/faults route, which ignores the global-only correlation flags,
  so the route advertises exactly what the handler reads.
- scripts/check_handlers_typed_query.sh: also ban has_param() and whitespace-
  evaded raw reads, scan both the source and include handler layers, and fail
  the gate on grep I/O errors instead of passing silently.
- quality.yml: run the typed-query gate in CI (format-lint job).
- tests: per-entity faults advertises status only; boolean spelling parsing.

Author: bburda

.github/workflows/quality.yml

@@ -78,6 +78,9 @@ jobs:
       - name: No naked rclcpp subscription APIs (issue #375 regression gate)
         run: ./scripts/check_no_naked_subscriptions.sh
 
+      - name: Handlers read query params via typed query<T>() (zero-query-params regression gate)
+        run: ./src/ros2_medkit_gateway/scripts/check_handlers_typed_query.sh
+
       - name: Show results
         if: always()
         run: colcon test-result --verbose

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

@@ -30,7 +30,8 @@ inline constexpr std::string_view kFaultSeverityLabelValues[] = {"INFO", "WARN",
 /// Fault aggregated status (fault_detail_schema - status.aggregatedStatus).
 inline constexpr std::string_view kFaultAggregatedStatusValues[] = {"active", "passive", "cleared"};
 
-/// Fault status query filter (FaultListQuery.status / FaultClearQuery.status).
+/// Fault status query filter (FaultListQuery.status / FaultEntityListQuery.status
+/// / FaultClearQuery.status).
 /// Mirrors the values parse_fault_status_param() accepts in http_utils.hpp;
 /// any other value yields ERR_INVALID_PARAMETER. The leading entry must be a
 /// handler-accepted value (the OpenAPI callability test sends enum[0]).

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

@@ -458,9 +458,10 @@ struct dto_sample<FaultClearResult> {
 };
 
 // =============================================================================
-// FaultListQuery - query parameters for GET /faults and GET /{entity}/faults.
-// Read by the handlers via TypedRequest::query<FaultListQuery>() and declared in
-// the OpenAPI spec via the same descriptor, so the two cannot drift.
+// FaultListQuery - query parameters for the global GET /faults list only. Read
+// by list_all_faults via TypedRequest::query<FaultListQuery>() and declared in
+// the OpenAPI spec via the same descriptor, so the two cannot drift. The
+// per-entity GET /{entity}/faults uses the narrower FaultEntityListQuery below.
 // =============================================================================
 struct FaultListQuery {
   std::optional<std::string> status;
@@ -476,6 +477,21 @@ inline constexpr auto dto_fields<FaultListQuery> = std::make_tuple(
     field("include_clusters", &FaultListQuery::include_clusters, Presence::kOptional,
           "Include fault clusters in the response"));
 
+// FaultEntityListQuery - query parameters for the per-entity GET /{entity}/faults
+// route. Only `status` applies: include_muted / include_clusters are honored
+// solely by the global GET /faults, because their correlation metadata is
+// computed across the whole fault manager and a scoped response would leak
+// cross-entity data. Declaring a narrower DTO here keeps the route advertising
+// exactly what the handler reads (no spec-vs-runtime drift on the declared axis).
+struct FaultEntityListQuery {
+  std::optional<std::string> status;
+};
+
+template <>
+inline constexpr auto dto_fields<FaultEntityListQuery> =
+    std::make_tuple(field_enum("status", &FaultEntityListQuery::status, kFaultStatusFilterValues,
+                               "Filter by fault status: pending, confirmed, cleared, healed, or all"));
+
 // FaultClearQuery - query parameters for DELETE /faults (clear all). Only the
 // status filter applies; the correlation flags are list-only.
 struct FaultClearQuery {

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/query.hpp

@@ -29,10 +29,22 @@
 // are what the spec advertises, a handler cannot read an undeclared query
 // parameter. Adding a parameter means adding a member - which appears in the
 // spec automatically.
+//
+// Scope of the guarantee: the descriptor binds parameter NAMES and PRESENCE -
+// those cannot drift between the spec and the parser. It does NOT police a
+// parameter's VALUE. The emitted `schema` (enum membership, type/format) is
+// advisory metadata; enforcing it (e.g. rejecting `?status=bogus`) is the
+// handler's job, where the richer 400 payload lives. To keep the two type
+// universes aligned, query fields are constrained at compile time to the scalar
+// shapes the parser supports (string / bool, optionally std::optional), and a
+// query parameter can never be `required` (the parser always tolerates absence,
+// leaving the member at its default).
 
+#include <cctype>
 #include <nlohmann/json.hpp>
 #include <optional>
 #include <string>
+#include <tuple>
 #include <type_traits>
 #include <utility>
 
@@ -56,16 +68,48 @@ struct query_value<std::optional<U>> {
 template <class M>
 using query_value_t = typename query_value<M>::type;
 
+/// The scalar shapes a query parameter may take (after std::optional unwrapping).
+/// This is the single source of truth shared by both sides of the contract: the
+/// reader (`assign_query_field`) parses exactly these, and the writer
+/// (`QueryParamWriter`) static_asserts every field against it - so the emitted
+/// schema can never advertise a type the parser would reject.
+template <class U>
+inline constexpr bool is_query_scalar_v = std::is_same_v<U, std::string> || std::is_same_v<U, bool>;
+
+/// True iff every field of the query DTO `T` is declared optional (never
+/// `Presence::kRequired`). The parser leaves absent parameters at their default
+/// and never rejects a missing query parameter, so a `required:true` parameter
+/// would be advertised but silently unenforced; this check makes that
+/// unrepresentable. Evaluated in a `static_assert` inside `QueryParamWriter`.
+template <class T>
+constexpr bool query_dto_all_optional() {
+  bool all_optional = true;
+  std::apply(
+      [&all_optional](const auto &... f) {
+        ((all_optional = all_optional && (f.presence != Presence::kRequired)), ...);
+      },
+      dto_fields<T>);
+  return all_optional;
+}
+
 /// Emits the OpenAPI `parameters` array (all `in: query`) for a query DTO `T`,
 /// derived from the same `dto_fields<T>` descriptor the typed reader consumes.
 template <class T>
 struct QueryParamWriter {
   static nlohmann::json parameters() {
+    static_assert(query_dto_all_optional<T>(),
+                  "query DTO fields must be optional: the parser tolerates absence and never enforces "
+                  "presence, so a required query parameter would be advertised but silently unenforced. "
+                  "Declare the member as std::optional<...> or pass Presence::kOptional.");
     nlohmann::json params = nlohmann::json::array();
     for_each_field<T>([&](const auto & f) {
       using FieldT = std::decay_t<decltype(f)>;
       static_assert(!is_opaque_object_field_v<FieldT>, "query DTOs must not contain opaque-object fields");
       using MemberT = std::decay_t<decltype(std::declval<T>().*(f.ptr))>;
+      static_assert(is_query_scalar_v<query_value_t<MemberT>>,
+                    "query DTO fields must be std::string or bool (optionally std::optional). The reader "
+                    "(assign_query_field) parses only these, so any other type would advertise a schema the "
+                    "parser cannot honour.");
 
       nlohmann::json param;
       param["name"] = std::string(f.key);
@@ -76,6 +120,8 @@ struct QueryParamWriter {
       }
       nlohmann::json schema = schema_of<query_value_t<MemberT>>();
       if (f.enum_count > 0) {
+        // Advisory only: the parser does not reject off-enum values (see the
+        // file header). The handler validates membership and owns the 400.
         nlohmann::json values = nlohmann::json::array();
         for (std::size_t i = 0; i < f.enum_count; ++i) {
           values.push_back(std::string(f.enum_values[i]));
@@ -89,18 +135,37 @@ struct QueryParamWriter {
   }
 };
 
+/// Parses a raw query-string value as a boolean, matching the `type: boolean`
+/// the schema advertises. `true`/`1` (case-insensitive) are truthy; every other
+/// value - including an empty flag-style `?x`, `false`, and `0` - is false. The
+/// parser does not reject malformed values (query parsing never 400s; see the
+/// file header), so a typo simply reads as false.
+inline bool parse_query_bool(const std::string & raw) {
+  std::string lowered;
+  lowered.reserve(raw.size());
+  for (char c : raw) {
+    lowered.push_back(static_cast<char>(std::tolower(static_cast<unsigned char>(c))));
+  }
+  return lowered == "true" || lowered == "1";
+}
+
 /// Assigns a raw query-string value to a typed query-DTO member. Only the scalar
 /// shapes a query parameter can take are supported; any other field type is a
 /// compile error so unsupported types fail loudly at the route, not silently at
-/// runtime.
+/// runtime. `QueryParamWriter` static_asserts the same type set, so the schema
+/// and the parser can never advertise different shapes.
 template <class M>
 void assign_query_field(M & member, const std::string & raw) {
-  if constexpr (std::is_same_v<M, std::string> || std::is_same_v<M, std::optional<std::string>>) {
+  // Gate on the SAME trait QueryParamWriter enforces, so the reader and the
+  // schema writer can never advertise different type sets - is_query_scalar_v is
+  // the one source of truth, consulted by both sides.
+  static_assert(is_query_scalar_v<query_value_t<M>>,
+                "assign_query_field: unsupported query field type (expected std::string or bool, optionally "
+                "std::optional). is_query_scalar_v is the shared source of truth QueryParamWriter also enforces.");
+  if constexpr (std::is_same_v<query_value_t<M>, std::string>) {
     member = raw;
-  } else if constexpr (std::is_same_v<M, bool> || std::is_same_v<M, std::optional<bool>>) {
-    member = (raw == "true");
-  } else {
-    static_assert(sizeof(M) == 0, "assign_query_field: unsupported query field type");
+  } else {  // query_value_t<M> is bool, guaranteed by the static_assert above
+    member = parse_query_bool(raw);
   }
 }
 

src/ros2_medkit_gateway/scripts/check_handlers_typed_query.sh

@@ -31,13 +31,55 @@
 set -euo pipefail
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-HANDLERS_DIR="$(cd "${SCRIPT_DIR}/../src/http/handlers" && pwd)"
+
+# Handler layers to scan. Both the HTTP and the core handler layers are covered,
+# on the source side (src/.../handlers, the .cpp bodies) AND the include side
+# (include/.../handlers, where inline handler methods live in .hpp) - a raw read
+# hidden in a header or a second handler layer would otherwise bypass the gate.
+# The plugin HTTP shim (src/plugins/plugin_http_types.cpp) is intentionally NOT
+# scanned: it is the sanctioned raw-httplib boundary the typed wrapper is built on.
+scan_dirs=()
+for rel in \
+  "../src/http/handlers" \
+  "../src/core/http/handlers" \
+  "../include/ros2_medkit_gateway/http/handlers" \
+  "../include/ros2_medkit_gateway/core/http/handlers"; do
+  abs="${SCRIPT_DIR}/${rel}"
+  if [[ -d "${abs}" ]]; then
+    scan_dirs+=("$(cd "${abs}" && pwd)")
+  fi
+done
+
+if [[ ${#scan_dirs[@]} -eq 0 ]]; then
+  echo "ERROR: no handler directories found to scan under ${SCRIPT_DIR}/../src or ../include."
+  exit 1
+fi
 
 # Path reads (req.path), header reads (req.header), and fan-out (raw_for_framework
 # for path/Authorization) are legitimate; only query-string reads are banned.
-pattern='\.query_param\(|->query_param\(|get_param_value\('
+# Match the accessor name directly (not "req." + name) so whitespace tricks like
+# `req . query_param (` cannot slip past, and ban has_param() too - it is the
+# presence half of a raw query read.
+pattern='(query_param|get_param_value|has_param)[[:space:]]*\('
+
+# Run the scan with grep's own status preserved: 0 = matches, 1 = none, >=2 = a
+# real error (unreadable file, bad path). A blanket `|| true` would mask exit 2
+# as "no matches", so an unscannable handler file would pass the gate silently;
+# treat >=2 as a hard failure of the gate itself instead.
+set +e
+raw_hits="$(grep -rnE "${pattern}" "${scan_dirs[@]}" --include='*.cpp' --include='*.hpp')"
+grep_rc=$?
+set -e
+if [[ ${grep_rc} -ge 2 ]]; then
+  echo "ERROR: grep failed (exit ${grep_rc}) while scanning handler dirs; cannot certify the gate."
+  exit 1
+fi
 
-hits="$(grep -rnE "${pattern}" "${HANDLERS_DIR}" --include='*.cpp' || true)"
+# Heuristic: drop matches that sit in a whole-line // comment. Inline trailing
+# comments and block/string occurrences are not stripped (a grep-level lint
+# cannot parse C++), but those are rare and a stray mention in a comment is
+# better flagged than a real read silently missed.
+hits="$(printf '%s' "${raw_hits}" | grep -vE '^[^:]*:[0-9]+:[[:space:]]*//' || true)"
 if [[ -n "${hits}" ]]; then
   echo "ERROR: handlers must read query parameters via TypedRequest::query<T>(), not raw query reads."
   echo "Offending lines:"

src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp

@@ -462,19 +462,18 @@ http::Result<dto::FaultListResult> FaultHandlers::list_faults(const http::TypedR
       return tl::make_unexpected(err);
     }
 
-    const auto q = req.query<dto::FaultListQuery>();
+    const auto q = req.query<dto::FaultEntityListQuery>();
     auto filter_result = read_fault_status_filter(q.status, json{{entity_info.id_field, entity_id}});
     if (!filter_result) {
       return tl::make_unexpected(filter_result.error());
     }
     const auto filter = *filter_result;
 
-    // Note: include_muted / include_clusters URL params are intentionally
-    // ignored on per-entity routes - the underlying service returns
-    // correlation metadata computed across the entire fault manager, so
-    // emitting it on a scoped response would leak cross-entity data
-    // (review finding N1). The global `GET /faults` route is the only place
-    // these flags are honored.
+    // Note: the per-entity query DTO (FaultEntityListQuery) deliberately omits
+    // include_muted / include_clusters - the underlying service returns
+    // correlation metadata computed across the entire fault manager, so emitting
+    // it on a scoped response would leak cross-entity data. The global
+    // `GET /faults` route is the only place these flags are declared and honored.
 
     auto fault_mgr = ctx_.node()->get_fault_manager();
 

src/ros2_medkit_gateway/src/http/rest_server.cpp

@@ -656,7 +656,7 @@ void RESTServer::setup_routes() {
         .summary(std::string("List faults for ") + et.singular)
         .description(std::string("Returns all active faults reported by this ") + et.singular + ".")
         .operation_id(std::string("list") + capitalize(et.singular) + "Faults")
-        .query<dto::FaultListQuery>();
+        .query<dto::FaultEntityListQuery>();
 
     reg.get<dto::FaultDetailResult>(entity_path + "/faults/{fault_code}",
                                     [this](http::TypedRequest req) -> http::Result<dto::FaultDetailResult> {

src/ros2_medkit_gateway/test/test_route_registry.cpp

@@ -54,6 +54,7 @@ inline constexpr std::string_view dto_name<RouteRegistryTestSeedDto> = "RouteReg
 }  // namespace ros2_medkit_gateway
 
 using namespace ros2_medkit_gateway::openapi;
+using ros2_medkit_gateway::dto::FaultEntityListQuery;
 using ros2_medkit_gateway::dto::FaultListQuery;
 using ros2_medkit_gateway::dto::RouteRegistryTestSeedDto;
 using ros2_medkit_gateway::http::Result;
@@ -210,6 +211,30 @@ TEST_F(RouteRegistryTest, TypedQueryDeclaresParametersFromDto) {
   EXPECT_FALSE((*include_muted)["required"].get<bool>());
 }
 
+// @verifies REQ_INTEROP_002
+TEST_F(RouteRegistryTest, PerEntityFaultsQueryAdvertisesStatusOnly) {
+  // The per-entity /faults route uses the narrower FaultEntityListQuery so the
+  // spec advertises exactly what the handler reads: status only. The
+  // correlation flags (include_muted / include_clusters) are global-only, so
+  // they must NOT appear on the per-entity route.
+  seed_get(registry_, "/apps/{app_id}/faults").tag("Faults").query<FaultEntityListQuery>();
+
+  auto paths = registry_.to_openapi_paths();
+  ASSERT_TRUE(paths.contains("/apps/{app_id}/faults"));
+  auto & get_op = paths["/apps/{app_id}/faults"]["get"];
+  ASSERT_TRUE(get_op.contains("parameters"));
+
+  std::vector<std::string> query_names;
+  for (const auto & p : get_op["parameters"]) {
+    if (p["in"] == "query") {
+      query_names.push_back(p["name"].get<std::string>());
+    }
+  }
+
+  ASSERT_EQ(query_names.size(), 1u);
+  EXPECT_EQ(query_names[0], "status");
+}
+
 // =============================================================================
 // to_regex_path - path conversion
 // =============================================================================

src/ros2_medkit_gateway/test/test_typed_router.cpp

@@ -183,6 +183,27 @@ TEST(TypedRouter_TypedRequest, TypedQueryLeavesAbsentParamsAtDefault) {
   EXPECT_FALSE(q.include_clusters);
 }
 
+TEST(TypedRouter_TypedRequest, TypedQueryParsesBooleanSpellings) {
+  // The schema advertises type: boolean; "true"/"1" (case-insensitive) are
+  // truthy, every other spelling - including "0", "false", and an empty
+  // flag-style value - parses as false.
+  auto parse_include_muted = [](const char * value) {
+    httplib::Request req;
+    req.path = "/api/v1/faults";
+    req.params.emplace("include_muted", value);
+    return TypedRequest(req).query<FaultListQuery>().include_muted;
+  };
+
+  EXPECT_TRUE(parse_include_muted("true"));
+  EXPECT_TRUE(parse_include_muted("TRUE"));
+  EXPECT_TRUE(parse_include_muted("True"));
+  EXPECT_TRUE(parse_include_muted("1"));
+  EXPECT_FALSE(parse_include_muted("false"));
+  EXPECT_FALSE(parse_include_muted("0"));
+  EXPECT_FALSE(parse_include_muted(""));
+  EXPECT_FALSE(parse_include_muted("yes"));
+}
+
 TEST(TypedRouter_TypedRequest, FanOutDisabledTrueOnlyWhenHeaderPresent) {
   {
     httplib::Request req;