refactor(firmware): simplify configuration and diagnostics

firmware/libs/kaptan-config-mavlink/Cargo.toml

@@ -10,5 +10,5 @@ workspace = true
 
 [dependencies]
 kaptan-config = { path = "../kaptan-config" }
-kaptan-log = { path = "../kaptan-log" }
+kaptan-err-propagate = { path = "../kaptan-err-propagate" }
 kaptan-mavlink = { path = "../kaptan-mavlink", default-features = false, features = ["dialect-kaptan"] }

firmware/libs/kaptan-config-mavlink/src/protocol.rs

@@ -1,7 +1,7 @@
 //! Stable typed outcomes for the Kaptan configuration microservice.
 
 use kaptan_config::{ConfigState, ConfigViolation, FieldId};
-use kaptan_log::KaptanError;
+use kaptan_err_propagate::KaptanError;
 
 /// Operation reported in an authoritative result record.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -36,28 +36,40 @@ pub enum ResultKind {
 #[derive(Clone, Copy, Debug, Eq, PartialEq, KaptanError)]
 pub enum ProtocolError {
     /// The request target did not match this component.
+    #[kaptan_error(id = 2001)]
     BadTarget,
     /// The configuration lease or single transaction slot is occupied.
+    #[kaptan_error(id = 2002)]
     SessionBusy,
     /// The session is unknown, expired, or belongs to a different principal.
+    #[kaptan_error(id = 2003)]
     InvalidSession,
     /// Reusing one request ID with different content is forbidden.
+    #[kaptan_error(id = 2004)]
     RequestConflict,
     /// The request was not based on the current transaction revision.
+    #[kaptan_error(id = 2005)]
     StaleRevision,
     /// A command is invalid in the current lifecycle state.
+    #[kaptan_error(id = 2006)]
     BadState,
     /// The field is not present in this schema.
+    #[kaptan_error(id = 2007)]
     UnknownField(FieldId),
     /// A query requested an empty or out-of-range descriptor span.
+    #[kaptan_error(id = 2008)]
     InvalidQuery,
     /// The typed setter does not match the field descriptor.
+    #[kaptan_error(id = 2009)]
     TypeMismatch(FieldId),
     /// A local or complete validation issue.
+    #[kaptan_error(id = 2010)]
     Violation(ConfigViolation),
     /// Persistence failed after a complete validation pass.
+    #[kaptan_error(id = 2011)]
     StorageFailure,
     /// The adapter reached an impossible internal state.
+    #[kaptan_error(id = 2012)]
     Internal,
 }
 

firmware/libs/kaptan-config-mavlink/src/service/session.rs

@@ -23,6 +23,12 @@ pub(super) struct Session {
     pub(super) revision: u32,
 }
 
+#[derive(Debug, kaptan_err_propagate::KaptanError)]
+enum CommitTransactionError<E> {
+    #[kaptan_error(id = 2201)]
+    Store(E),
+}
+
 /// Owns only the one GCS transaction lease and bounded protocol state.
 ///
 /// This protocol never accepts raw frames or interprets transport authority
@@ -303,25 +309,11 @@ impl ConfigProtocol {
                 }
                 Err(CommitError::Store(_)) => self.rejected(command, ProtocolError::Internal),
             },
-            ConfigCommand::Commit { request_id, .. } => match manager.commit() {
-                Ok(()) => {
-                    self.session = None;
-                    self.refresh(manager);
-                    self.reply(
-                        request_id,
-                        0,
-                        session.revision,
-                        Operation::Commit,
-                        ResultKind::CommittedRestartRequired,
-                        None,
-                    )
-                }
-                Err(CommitError::NoTransaction) => self.rejected(command, ProtocolError::BadState),
-                Err(CommitError::InvalidCandidate(violation)) => {
-                    self.rejected(command, ProtocolError::Violation(violation))
-                }
-                Err(CommitError::Store(_)) => self.rejected(command, ProtocolError::StorageFailure),
-            },
+            ConfigCommand::Commit { request_id, .. } => self
+                .commit_transaction(manager, command, request_id, session)
+                .unwrap_or_else(|CommitTransactionError::Store(_)| {
+                    self.rejected(command, ProtocolError::StorageFailure)
+                }),
             ConfigCommand::Abort { request_id, .. } => {
                 manager.abort();
                 self.session = None;
@@ -343,6 +335,43 @@ impl ConfigProtocol {
         }
     }
 
+    /// Classifies expected configuration rejection separately from a terminal
+    /// persistence failure. Only the latter leaves this boundary and becomes
+    /// a typed firmware error; the configuration result remains authoritative
+    /// for the requesting user.
+    #[kaptan_err_propagate::propagate]
+    fn commit_transaction<C, S>(
+        &mut self,
+        manager: &mut ConfigManager<C, S>,
+        command: ConfigCommand,
+        request_id: u32,
+        session: Session,
+    ) -> Result<ConfigReply, CommitTransactionError<S::Error>>
+    where
+        C: ConfigModel,
+        S: ConfigStore<C>,
+    {
+        match manager.commit() {
+            Ok(()) => {
+                self.session = None;
+                self.refresh(manager);
+                Ok(self.reply(
+                    request_id,
+                    0,
+                    session.revision,
+                    Operation::Commit,
+                    ResultKind::CommittedRestartRequired,
+                    None,
+                ))
+            }
+            Err(CommitError::NoTransaction) => Ok(self.rejected(command, ProtocolError::BadState)),
+            Err(CommitError::InvalidCandidate(violation)) => {
+                Ok(self.rejected(command, ProtocolError::Violation(violation)))
+            }
+            Err(CommitError::Store(error)) => Err(CommitTransactionError::Store(error)),
+        }
+    }
+
     fn allocate_session_id(&mut self) -> Option<u32> {
         let id = self.next_session_id?;
         self.next_session_id = id.get().checked_add(1).and_then(NonZeroU32::new);
@@ -646,6 +675,21 @@ mod tests {
         }
     }
 
+    struct FailingStore(Rc<Cell<u32>>);
+
+    impl ConfigStore<Candidate> for FailingStore {
+        type Error = ();
+
+        fn load(&mut self) -> Result<Option<Candidate>, Self::Error> {
+            Ok(None)
+        }
+
+        fn persist(&mut self, _: &Candidate) -> Result<(), Self::Error> {
+            self.0.set(self.0.get() + 1);
+            Err(())
+        }
+    }
+
     /// Test-only composition root. Production code must retain these two
     /// objects separately; this wrapper keeps protocol examples compact while
     /// exercising the same externally-owned-manager dispatch API.
@@ -1189,6 +1233,66 @@ mod tests {
         assert_eq!(next_owner.unwrap().result, ResultKind::Accepted);
     }
 
+    #[test]
+    fn failed_commit_reports_the_returned_error_only_once() {
+        while kaptan_err_propagate::take().is_some() {}
+        let persists = Rc::new(Cell::new(0));
+        let store = FailingStore(Rc::clone(&persists));
+        let mut manager = ConfigManager::new(Candidate(2), (), store).unwrap();
+        let mut protocol = ConfigProtocol::new(Target::new(1, 1));
+        let mut begin_reply = None;
+        protocol
+            .dispatch(
+                &mut manager,
+                Authorized::host(
+                    7,
+                    ConfigCommand::Begin {
+                        request_id: 1,
+                        target: Target::new(1, 1),
+                    },
+                ),
+                |response| {
+                    begin_reply = Some(result(response));
+                    Ok::<(), core::convert::Infallible>(())
+                },
+            )
+            .unwrap();
+        let commit = ConfigCommand::Commit {
+            request_id: 2,
+            session_id: begin_reply.unwrap().session_id,
+            expected_revision: 0,
+            target: Target::new(1, 1),
+        };
+
+        let mut first_reply = None;
+        protocol
+            .dispatch(&mut manager, Authorized::host(7, commit), |response| {
+                first_reply = Some(result(response));
+                Ok::<(), core::convert::Infallible>(())
+            })
+            .unwrap();
+        assert_eq!(persists.get(), 1);
+        assert_eq!(
+            first_reply.unwrap().error,
+            Some(ProtocolError::StorageFailure)
+        );
+        let error =
+            kaptan_err_propagate::take().expect("failed commit must report its returned error");
+        assert_eq!(error.id(), 2201);
+        while kaptan_err_propagate::take().is_some() {}
+
+        protocol
+            .dispatch(&mut manager, Authorized::host(7, commit), |_| {
+                Ok::<(), core::convert::Infallible>(())
+            })
+            .unwrap();
+        assert_eq!(persists.get(), 1);
+        assert!(
+            kaptan_err_propagate::take().is_none(),
+            "a cached command replay must not report the error again"
+        );
+    }
+
     #[test]
     fn configuration_result_remains_the_structured_failure_authority() {
         let manager = ConfigManager::new(Candidate(2), (), Store).unwrap();

firmware/libs/kaptan-config-mavlink/src/wire.rs

@@ -3,7 +3,7 @@
 use kaptan_config::{
     ConfigState, ConfigValue, FieldDescriptor, FieldId, ValidationRule, ValueKind,
 };
-use kaptan_log::KaptanError;
+use kaptan_err_propagate::KaptanError;
 use kaptan_mavlink::message::Message;
 use kaptan_mavlink::message::kaptan::{
     KAPTAN_CONFIG_DESCRIPTOR_DATA, KAPTAN_CONFIG_ENUM_OPTION_DATA, KAPTAN_CONFIG_INFO_DATA,
@@ -67,14 +67,19 @@ where
 #[derive(Clone, Copy, Debug, Eq, PartialEq, KaptanError)]
 pub enum DecodeError {
     /// The message belongs to a response, telemetry, or another service.
+    #[kaptan_error(id = 2101)]
     UnsupportedMessage,
     /// A wire field identity used a reserved sentinel.
+    #[kaptan_error(id = 2102)]
     InvalidFieldId,
     /// The message used an unknown value-source discriminator.
+    #[kaptan_error(id = 2103)]
     InvalidValueSource,
     /// A boolean setter used a representation other than zero or one.
+    #[kaptan_error(id = 2104)]
     InvalidBoolean,
     /// A configuration request used a broadcast/zero target endpoint.
+    #[kaptan_error(id = 2105)]
     InvalidTarget,
 }