From d22bbc4ab2d2f0f24530baf174607988e469c7fc Mon Sep 17 00:00:00 2001
From: Raphael Maenle <raphael@maenle.net>
Date: Fri, 26 Jun 2026 21:48:29 +0200
Subject: [PATCH] fix(daemon)+fix(client): close four fool-proofing gaps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

C++ (equipment_service.hpp):
- ReportSubstrate ARRIVED: check CreateResult and return INVALID_OBJECT
  when the substrate ID already exists, instead of silently doing nothing
- ReportSubstrate/ReportModule default switch branches: return false
  (→ CANNOT_DO_NOW) for unknown enum values instead of silently accepting

Python (_client.py):
- @eq.command: raise NameError (client-side name validation) instead of
  SecsGemError (which means "daemon declined a request") — wrong type
- Module docstring: update example to show @eq.command / eq.names API

Test (test_daemon_service.cpp):
- Add duplicate-ARRIVED check (expects INVALID_OBJECT)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 clients/python/secsgem_client/_client.py     | 11 +++++------
 include/secsgem/daemon/equipment_service.hpp |  7 ++++---
 tests/test_daemon_service.cpp                |  2 ++
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/clients/python/secsgem_client/_client.py b/clients/python/secsgem_client/_client.py
index f25a7bc..eae842e 100644
--- a/clients/python/secsgem_client/_client.py
+++ b/clients/python/secsgem_client/_client.py
@@ -10,11 +10,12 @@ equipment.yaml; values are plain Python.
 
     eq = Equipment("localhost:50051")
     eq.set(ChamberPressure=2.5)
-    eq.fire("ProcessStarted")
+    eq.fire(eq.names.event.ProcessStarted)   # typo-safe; plain strings also work
 
-    @eq.on("START")
-    def start(cmd):
+    @eq.command                              # function name IS the RCMD name,
+    def START(cmd):                          # validated against equipment at import
         run_recipe(cmd.params.get("PPID"))
+        eq.fire(eq.names.event.ProcessStarted)
 
     eq.listen()   # blocks, dispatching host commands to your handlers
 """
@@ -238,9 +239,7 @@ class Equipment:
         if name not in self.names.command:
             close = difflib.get_close_matches(name, dir(self.names.command), n=3)
             hint = f" Did you mean {', '.join(close)}?" if close else ""
-            raise SecsGemError(
-                pb.Ack.PARAMETER_INVALID,
-                f"no host command '{name}' to bind @eq.command to.{hint}")
+            raise NameError(f"no host command '{name}' to bind @eq.command to.{hint}")
         self._handlers[name] = fn
         return fn
 
diff --git a/include/secsgem/daemon/equipment_service.hpp b/include/secsgem/daemon/equipment_service.hpp
index 654f20c..83c55ab 100644
--- a/include/secsgem/daemon/equipment_service.hpp
+++ b/include/secsgem/daemon/equipment_service.hpp
@@ -536,7 +536,8 @@ class EquipmentService final : public pb::Equipment::Service {
     auto outcome = rt_.read_sync([this, sid, cid, slot, m]() -> std::optional<bool> {
       auto& subs = rt_.model().substrates;
       if (m == pb::SubstrateReport::ARRIVED) {
-        subs.create(sid, cid, slot);  // AtSource / NeedsProcessing
+        if (subs.create(sid, cid, slot) == gem::SubstrateStore::CreateResult::Denied_AlreadyExists)
+          return std::nullopt;  // → INVALID_OBJECT: duplicate substrate ID
         return true;
       }
       if (!subs.has(sid)) return std::nullopt;
@@ -550,7 +551,7 @@ class EquipmentService final : public pb::Equipment::Service {
         case pb::SubstrateReport::PROCESSED:
           return subs.fire_processing_event(sid, gem::SubstrateProcessingEvent::EndProcessing);
         default:
-          return true;
+          return false;  // unknown milestone
       }
     });
     ack_from_outcome(outcome, resp, "substrate '" + sid + "'",
@@ -575,7 +576,7 @@ class EquipmentService final : public pb::Equipment::Service {
         case pb::ModuleReport::NOT_EXECUTING:
           return mods.fire(mid, gem::ModuleEvent::Reset);
         default:
-          return true;
+          return false;  // unknown state
       }
     });
     // Module auto-creates, so "not found" can't happen; only the FSM verdict.
diff --git a/tests/test_daemon_service.cpp b/tests/test_daemon_service.cpp
index 8d715de..5621cdc 100644
--- a/tests/test_daemon_service.cpp
+++ b/tests/test_daemon_service.cpp
@@ -774,6 +774,8 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") {
 
   // Reporting on a substrate that never ARRIVED is rejected.
   CHECK(sub("WFR-GHOST", pb::SubstrateReport::AT_WORK) == pb::Ack::INVALID_OBJECT);
+  // Duplicate ARRIVED (same substrate ID already in the store) is rejected.
+  CHECK(sub("WFR-1", pb::SubstrateReport::ARRIVED) == pb::Ack::INVALID_OBJECT);
 
   auto mod = [&](const std::string& mid, pb::ModuleReport::State st) {
     grpc::ClientContext ctx;