config: multi-error YAML validator + --validate-config CLI flag

The existing loader throws ConfigError on the first problem it hits. A customer with a tool-specific equipment.yaml that has six issues sees one, fixes, restarts, sees the next, fixes, restarts — six edit-restart cycles before the server even binds. Day-1 friction is the top support ticket source in fab integrations. This commit adds a parallel validator that does a separate read-only pass and surfaces *every* issue at once: $ secs_server --validate-config \ --config equipment.yaml \ --state-table control_state.yaml [error] equipment.yaml:5 svids[0].type — unknown SECS-II type `WTF` [error] equipment.yaml:7 alarms[0].category — value 200 out of range [0, 127] [error] equipment.yaml:9 host_commands[0].emit_ceid — CEID 999 not declared in `ceids` section 3 error(s), 0 warning(s) across 4 files What it catches: - Missing required fields (device.model_name, .software_rev, …) - Range violations (alarm category must be 0–127, spool streams 1–127, device.id fits u16, etc.) - Unknown enum values (SECS-II types, HCACK values, control/PJ/CJ state and event names — using the right case + snake convention the runtime parsers enforce) - Duplicate IDs within svids / dvids / ecids / ceids / alarms, duplicate PPIDs in recipes, duplicate command names in host_commands - Referential integrity: host_commands[*].emit_ceid must exist in ceids; host_commands[*].set_alarm must exist in alarms; emit_on_control_change must exist in ceids - PJ-table-specific: `NoState` sentinel rejected as `initial`, `from`, or `to` (matches loader's existing runtime check) - yaml-cpp Mark → 1-based line numbers when available What it doesn't catch (out of scope this round): - JSON Schema for editor red-squigglies (future) - Deep semantic checks across state-table reachability - ECID min/max value parsing (would need numeric type coupling) Tests cover: clean file passes; multi-error YAML surfaces every issue on a single pass; line numbers populate; control_state / process_job_state / control_job_state casing conventions; format_issues_to renders both severities; the shipped data/equipment.yaml etc. validate cleanly (regression tripwire if anyone breaks the demo configs). INTEGRATION.md §2.3 calls out the flag and suggests CI use. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-09 14:32:09 +02:00
parent d73906f372
commit a4599b3b9d
6 changed files with 833 additions and 5 deletions
@@ -0,0 +1,204 @@
+// Multi-error config validator tests.  Crafts intentionally broken
+// YAML, runs the validator, and asserts that the issues vector
+// surfaces every problem on a single pass — that's the contract the
+// --validate-config CLI flag relies on.
+
+#include <doctest/doctest.h>
+
+#include <algorithm>
+#include <filesystem>
+#include <fstream>
+#include <random>
+#include <sstream>
+#include <string>
+
+#include "secsgem/config/validate.hpp"
+
+namespace fs = std::filesystem;
+using secsgem::config::ConfigIssue;
+using secsgem::config::ConfigValidator;
+
+namespace {
+
+fs::path scratch_path(const char* tag, const char* ext) {
+  std::random_device rd;
+  auto dir = fs::temp_directory_path() /
+             (std::string("secsgem-cfg-") + tag + "-" + std::to_string(rd()));
+  fs::create_directories(dir);
+  return dir / (std::string("file.") + ext);
+}
+
+void write(const fs::path& p, const std::string& contents) {
+  std::ofstream out(p);
+  out << contents;
+}
+
+bool any_match(const std::vector<ConfigIssue>& issues,
+                const std::string& field_substr) {
+  return std::any_of(issues.begin(), issues.end(), [&](const ConfigIssue& i) {
+    return i.field_path.find(field_substr) != std::string::npos;
+  });
+}
+
+}  // namespace
+
+TEST_CASE("Validate: equipment.yaml — clean file produces zero errors") {
+  auto p = scratch_path("clean", "yaml");
+  write(p, R"YAML(
+device:
+  id: 0
+  model_name: "TEST"
+  software_rev: "0.1"
+ceids:
+  - {id: 300, name: ProcessStarted}
+alarms:
+  - {id: 1, text: "high temp", category: 4}
+recipes:
+  - {id: "R-1", body: "STEP HEAT"}
+host_commands:
+  - {name: START, ack: Accept, emit_ceid: 300, set_alarm: 1}
+)YAML");
+  ConfigValidator v;
+  CHECK(v.validate_equipment(p.string()));
+  CHECK(v.error_count() == 0);
+}
+
+TEST_CASE("Validate: equipment.yaml — multiple errors surface in one pass") {
+  auto p = scratch_path("multi", "yaml");
+  write(p, R"YAML(
+device:
+  id: 0
+  model_name: "TEST"
+  # software_rev: missing
+svids:
+  - {id: 1, name: ControlState, type: ASCII, value: ""}
+  - {id: 1, name: Duplicate, type: U4, value: 0}
+ecids:
+  - {id: 10, name: BadType, type: WTF, value: 1}
+alarms:
+  - {id: 1, text: "x", category: 200}  # out of range (max 127)
+host_commands:
+  - {name: START, ack: NotARealAck, emit_ceid: 999, set_alarm: 999}
+emit_on_control_change: 12345
+)YAML");
+  ConfigValidator v;
+  CHECK_FALSE(v.validate_equipment(p.string()));
+  const auto& issues = v.issues();
+  // We expect at least: missing software_rev, duplicate SVID id,
+  // unknown type WTF, alarm category out of range, unknown HCACK,
+  // CEID 999 not declared, ALID 999 not declared, emit_on_control_change CEID undeclared.
+  CHECK(v.error_count() >= 8);
+  CHECK(any_match(issues, "device.software_rev"));
+  CHECK(any_match(issues, "svids[1].id"));
+  CHECK(any_match(issues, "ecids[0].type"));
+  CHECK(any_match(issues, "alarms[0].category"));
+  CHECK(any_match(issues, "host_commands[0].ack"));
+  CHECK(any_match(issues, "host_commands[0].emit_ceid"));
+  CHECK(any_match(issues, "host_commands[0].set_alarm"));
+  CHECK(any_match(issues, "emit_on_control_change"));
+}
+
+TEST_CASE("Validate: equipment.yaml — line numbers populate when known") {
+  auto p = scratch_path("lines", "yaml");
+  write(p, R"YAML(
+device:
+  id: 0
+  model_name: "TEST"
+  software_rev: "0.1"
+alarms:
+  - {id: 1, text: "ok", category: 4}
+  - {id: 1, text: "dup", category: 4}
+)YAML");
+  ConfigValidator v;
+  v.validate_equipment(p.string());
+  bool found_lineno = false;
+  for (const auto& i : v.issues()) {
+    if (i.field_path.find("alarms[1]") != std::string::npos && i.line) {
+      found_lineno = true;
+      // line 8 in our heredoc (the second alarm).
+      CHECK(*i.line >= 7);
+      CHECK(*i.line <= 9);
+    }
+  }
+  CHECK(found_lineno);
+}
+
+TEST_CASE("Validate: control_state.yaml — unknown state name") {
+  auto p = scratch_path("ctrl", "yaml");
+  write(p, R"YAML(
+initial: HostOffline
+transitions:
+  - {from: NotAState, on: host_request_online, to: OnlineRemote, ack: Accept}
+  - {from: HostOffline, on: not_an_event, to: HostOffline}
+)YAML");
+  ConfigValidator v;
+  CHECK_FALSE(v.validate_control_state(p.string()));
+  const auto& issues = v.issues();
+  CHECK(any_match(issues, "transitions[0].from"));
+  CHECK(any_match(issues, "transitions[1].on"));
+}
+
+TEST_CASE("Validate: process_job_state.yaml — NoState sentinel rejected") {
+  auto p = scratch_path("pj", "yaml");
+  write(p, R"YAML(
+initial: NoState
+transitions:
+  - {from: NoState, on: select, to: SettingUp}
+  - {from: Queued, on: select, to: NoState}
+)YAML");
+  ConfigValidator v;
+  CHECK_FALSE(v.validate_process_job_state(p.string()));
+  // All three NoState placements should each surface as an error.
+  std::size_t no_state_hits = 0;
+  for (const auto& i : v.issues()) {
+    if (i.message.find("NoState") != std::string::npos) ++no_state_hits;
+  }
+  CHECK(no_state_hits == 3);
+}
+
+TEST_CASE("Validate: control_job_state.yaml — clean file passes") {
+  auto p = scratch_path("cj-clean", "yaml");
+  // State names are CamelCase, event names are snake_case — the
+  // convention the runtime parsers in src/gem/control_job_state.cpp
+  // enforce.
+  write(p, R"YAML(
+initial: Queued
+transitions:
+  - {from: Queued, on: select, to: Selected, ack: Accept}
+  - {from: Selected, on: start, to: Executing}
+)YAML");
+  ConfigValidator v;
+  CHECK(v.validate_control_job_state(p.string()));
+  CHECK(v.error_count() == 0);
+}
+
+TEST_CASE("Validate: format_issues_to renders error and warning rows") {
+  std::vector<ConfigIssue> issues = {
+      {"a.yaml", 12, "device.id", "out of range",
+       ConfigIssue::Severity::Error},
+      {"b.yaml", std::nullopt, "spool.max_size", "consider raising",
+       ConfigIssue::Severity::Warning},
+  };
+  std::ostringstream os;
+  secsgem::config::format_issues_to(os, issues);
+  const auto out = os.str();
+  CHECK(out.find("[error] a.yaml:12 device.id") != std::string::npos);
+  CHECK(out.find("[warn]  b.yaml spool.max_size") != std::string::npos);
+}
+
+TEST_CASE("Validate: ships data/equipment.yaml without errors") {
+  // The shipped demo config is the canonical "good" sample; if anyone
+  // edits data/equipment.yaml in a way that breaks the validator's
+  // invariants, this test fails loudly.
+  ConfigValidator v;
+  v.validate_equipment(SECSGEM_DATA_DIR "/equipment.yaml");
+  v.validate_control_state(SECSGEM_DATA_DIR "/control_state.yaml");
+  v.validate_process_job_state(SECSGEM_DATA_DIR "/process_job_state.yaml");
+  v.validate_control_job_state(SECSGEM_DATA_DIR "/control_job_state.yaml");
+  if (v.has_errors()) {
+    std::ostringstream os;
+    secsgem::config::format_issues_to(os, v.issues());
+    FAIL(os.str());
+  }
+  CHECK(v.error_count() == 0);
+}