[Controller] Preserve granular commissioning-failure context in CompletionStatus (#72228)

* Preserve granular commissioning-failure context in CompletionStatus

Today OnCommissioningFailure collapses several distinguishable failure
modes into a single CHIP_ERROR, leaving controllers unable to
disambiguate without parsing logs. This change threads structured
failure context through the commissioning pipeline so consumers can
branch on the device-reported cause.

DevicePairingDelegate:
- New OnCommissioningFailure(PeerId, const CompletionStatus &) overload
  whose default implementation forwards to the legacy 4-arg form.
  Existing subclasses keep working unchanged.

CompletionStatus widened with optional structured fields:
- commissioningError - GeneralCommissioning::CommissioningErrorEnum
- networkCommissioningStatus + connectNetworkErrorValue
- operationalCertStatus - NodeOperationalCertStatusEnum from NOCResponse
- commissioningDebugText, networkCommissioningDebugText - owned
  std::string copies of device-supplied debug text

Three report variants updated to carry the additional data:
- CommissioningErrorInfo gains std::string debugText
- NetworkCommissioningStatusInfo gains std::string debugText +
  Optional<int32_t> connectNetworkErrorValue
- New OperationalCertErrorInfo carrying the cert-status enum

AutoCommissioner::CommissioningStepFinished populates CompletionStatus
from these report variants on the failure path.

CHIPDeviceController response handlers populate the corresponding
reports: OnArmFailSafe, OnSetRegulatoryConfigResponse,
OnSetTCAcknowledgementsResponse, OnCommissioningCompleteResponse,
OnOperationalCertificateAddResponse, OnConnectNetworkResponse,
OnNetworkConfigResponse.

PASESession:
- OnFailureStatusReport maps kProtocolCodeNoSharedRoot to
  CHIP_ERROR_NO_SHARED_TRUSTED_ROOT (PASE has no shared-root semantics,
  so a peer sending this is misconfigured, but the mapping is more
  useful than CHIP_ERROR_INTERNAL).
- OnFailureStatusReport maps kProtocolCodeBusy to CHIP_ERROR_BUSY
  (uncommon in PASE but not spec-forbidden).

Backward compatibility:
- New OnCommissioningFailure overload defaults to forwarding to the
  legacy 4-arg form. Existing subclasses keep compiling and running.
- All new fields on CompletionStatus are Optional or std::string with
  empty as the absence marker.
- No public API removed.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: woody-apple

src/controller/AutoCommissioner.cpp

@@ -800,13 +800,27 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
         }
         else if (report.Is<CommissioningErrorInfo>())
         {
-            completionStatus.commissioningError = MakeOptional(report.Get<CommissioningErrorInfo>().commissioningError);
+            const auto & commissioningInfo          = report.Get<CommissioningErrorInfo>();
+            completionStatus.commissioningError     = MakeOptional(commissioningInfo.commissioningError);
+            completionStatus.commissioningDebugText = commissioningInfo.debugText;
+        }
+        else if (report.Is<OperationalCertErrorInfo>())
+        {
+            // Preserve the NodeOperationalCertStatusEnum from the device's NOCResponse
+            // (kInvalidPublicKey, kInvalidNodeOpId, kInvalidNOC, kFabricConflict, kLabelConflict,
+            // kInvalidFabricIndex, etc.) so callers can distinguish without losing fidelity to a
+            // generic CHIP_ERROR.
+            completionStatus.operationalCertStatus = MakeOptional(report.Get<OperationalCertErrorInfo>().operationalCertStatus);
         }
         else if (report.Is<NetworkCommissioningStatusInfo>())
         {
             // This report type is used when an error happens in either NetworkConfig or ConnectNetwork commands
-            completionStatus.networkCommissioningStatus =
-                MakeOptional(report.Get<NetworkCommissioningStatusInfo>().networkCommissioningStatus);
+            const auto & networkInfo                    = report.Get<NetworkCommissioningStatusInfo>();
+            completionStatus.networkCommissioningStatus = MakeOptional(networkInfo.networkCommissioningStatus);
+            // Preserve the optional ConnectNetworkResponse.errorValue (driver-level detail
+            // distinct from the spec-level networkingStatus). Null for NetworkConfigResponse.
+            completionStatus.connectNetworkErrorValue      = networkInfo.connectNetworkErrorValue;
+            completionStatus.networkCommissioningDebugText = networkInfo.debugText;
 
             // If we are configured to scan networks, then don't error out.
             // Instead, allow the app to try another network.

src/controller/CHIPDeviceController.cpp

@@ -1872,7 +1872,22 @@ void DeviceCommissioner::OnOperationalCertificateAddResponse(
     if (err != CHIP_NO_ERROR)
     {
         ChipLogProgress(Controller, "Add NOC failed with error: %" CHIP_ERROR_FORMAT, err.Format());
-        commissioner->CommissioningStageComplete(err);
+        // Preserve the device-reported NodeOperationalCertStatusEnum (kInvalidPublicKey,
+        // kInvalidNodeOpId, kInvalidNOC, kFabricConflict, kLabelConflict, kInvalidFabricIndex,
+        // kTableFull, etc.) in the report so OnCommissioningFailure consumers can distinguish
+        // these without losing fidelity to the generic CHIP_ERROR returned by
+        // ConvertFromOperationalCertStatus.
+        CommissioningDelegate::CommissioningReport report;
+        // ConvertFromOperationalCertStatus succeeds on kOk and returns a non-success CHIP_ERROR
+        // for any other value, but `err` at this point can also be a downstream local failure
+        // (e.g. from OnOperationalCredentialsProvisioningCompletion) while data.statusCode is
+        // still kOk. Guard so we never publish a "success enum" alongside a non-success err to
+        // OnCommissioningFailure consumers.
+        if (data.statusCode != OperationalCredentials::NodeOperationalCertStatusEnum::kOk)
+        {
+            report.Set<OperationalCertErrorInfo>(data.statusCode);
+        }
+        commissioner->CommissioningStageComplete(err, report);
     }
 }
 
@@ -2172,10 +2187,7 @@ void DeviceCommissioner::SendCommissioningCompleteCallbacks(NodeId nodeId, const
     }
     else
     {
-        // TODO: We should propogate detailed error information (commissioningError, networkCommissioningStatus) from
-        // completionStatus.
-        mPairingDelegate->OnCommissioningFailure(peerId, completionStatus.err, completionStatus.failedStage.ValueOr(kError),
-                                                 completionStatus.attestationResult);
+        mPairingDelegate->OnCommissioningFailure(peerId, completionStatus);
     }
 }
 
@@ -2975,7 +2987,9 @@ void DeviceCommissioner::OnArmFailSafe(void * context,
     if (data.errorCode != GeneralCommissioning::CommissioningErrorEnum::kOk)
     {
         err = CHIP_ERROR_INTERNAL;
-        report.Set<CommissioningErrorInfo>(data.errorCode);
+        // Preserve the device-supplied debugText so failure consumers can disambiguate
+        // ambiguous error codes (e.g. kBusyWithOtherAdmin specifics).
+        report.Set<CommissioningErrorInfo>(data.errorCode, data.debugText);
     }
 
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
@@ -2992,7 +3006,7 @@ void DeviceCommissioner::OnSetRegulatoryConfigResponse(
     if (data.errorCode != GeneralCommissioning::CommissioningErrorEnum::kOk)
     {
         err = CHIP_ERROR_INTERNAL;
-        report.Set<CommissioningErrorInfo>(data.errorCode);
+        report.Set<CommissioningErrorInfo>(data.errorCode, data.debugText);
     }
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
     commissioner->CommissioningStageComplete(err, report);
@@ -3008,6 +3022,7 @@ void DeviceCommissioner::OnSetTCAcknowledgementsResponse(
     if (data.errorCode != GeneralCommissioning::CommissioningErrorEnum::kOk)
     {
         err = CHIP_ERROR_INTERNAL;
+        // SetTCAcknowledgementsResponse has no debugText field per spec.
         report.Set<CommissioningErrorInfo>(data.errorCode);
     }
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
@@ -3099,7 +3114,9 @@ void DeviceCommissioner::OnNetworkConfigResponse(void * context,
     if (data.networkingStatus != NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess)
     {
         err = CHIP_ERROR_INTERNAL;
-        report.Set<NetworkCommissioningStatusInfo>(data.networkingStatus);
+        // Preserve debugText alongside the status enum so callers can distinguish
+        // ambiguous statuses (e.g. kAuthFailure: "wrong password" vs "regulatory restriction").
+        report.Set<NetworkCommissioningStatusInfo>(data.networkingStatus, data.debugText.ValueOr(CharSpan{}));
     }
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
     commissioner->CommissioningStageComplete(err, report);
@@ -3115,7 +3132,12 @@ void DeviceCommissioner::OnConnectNetworkResponse(
     if (data.networkingStatus != NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess)
     {
         err = CHIP_ERROR_INTERNAL;
-        report.Set<NetworkCommissioningStatusInfo>(data.networkingStatus);
+        // Preserve debugText alongside the status enum (see OnNetworkConfigResponse). Also
+        // surface the device-specific errorValue which carries driver-level failure detail
+        // (TX-power-limited / interference / association-failure code) distinct from the
+        // spec-level networkingStatus enum.
+        Optional<int32_t> errorValue = data.errorValue.IsNull() ? NullOptional : MakeOptional(data.errorValue.Value());
+        report.Set<NetworkCommissioningStatusInfo>(data.networkingStatus, data.debugText.ValueOr(CharSpan{}), errorValue);
     }
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
     commissioner->CommissioningStageComplete(err, report);
@@ -3131,7 +3153,7 @@ void DeviceCommissioner::OnCommissioningCompleteResponse(
     if (data.errorCode != GeneralCommissioning::CommissioningErrorEnum::kOk)
     {
         err = CHIP_ERROR_INTERNAL;
-        report.Set<CommissioningErrorInfo>(data.errorCode);
+        report.Set<CommissioningErrorInfo>(data.errorCode, data.debugText);
     }
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
     commissioner->CommissioningStageComplete(err, report);

src/controller/CommissioningDelegate.h

@@ -26,11 +26,14 @@
 #include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
 #include <credentials/jcm/TrustVerification.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <lib/support/CharSpanToStdString.h>
 #include <lib/support/Span.h>
 #include <lib/support/Variant.h>
 #include <matter/tracing/build_config.h>
 #include <system/SystemClock.h>
 
+#include <string>
+
 namespace chip {
 namespace Controller {
 
@@ -137,11 +140,30 @@ struct NOCChainGenerationParameters
 struct CompletionStatus
 {
     CompletionStatus() : err(CHIP_NO_ERROR), failedStage(NullOptional), attestationResult(NullOptional) {}
+
     CHIP_ERROR err;
     Optional<CommissioningStage> failedStage;
     Optional<Credentials::AttestationVerificationResult> attestationResult;
     Optional<app::Clusters::GeneralCommissioning::CommissioningErrorEnum> commissioningError;
     Optional<app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum> networkCommissioningStatus;
+    /// Optional device-reported low-level error from ConnectNetworkResponse.errorValue (e.g.
+    /// driver-specific TX-power-limited / interference / association-failure code), distinct
+    /// from the spec-level NetworkCommissioningStatusEnum already captured above.
+    Optional<int32_t> connectNetworkErrorValue;
+    /// Optional NodeOperationalCertStatusEnum from the device's NOCResponse during the
+    /// AddNOC / UpdateNOC / RemoveFabric flows. Lets callers distinguish kInvalidPublicKey
+    /// from kInvalidNodeOpId from kFabricConflict from kLabelConflict, etc., without losing
+    /// fidelity to a generic CHIP_ERROR.
+    Optional<app::Clusters::OperationalCredentials::NodeOperationalCertStatusEnum> operationalCertStatus;
+
+    /// Owned copy of the device-supplied GeneralCommissioning debugText from
+    /// ArmFailSafeResponse / SetRegulatoryConfigResponse / CommissioningCompleteResponse.
+    /// Empty if no debug text was provided.
+    std::string commissioningDebugText;
+
+    /// Owned copy of the device-supplied NetworkCommissioning debugText from
+    /// NetworkConfigResponse / ConnectNetworkResponse. Empty if no debug text was provided.
+    std::string networkCommissioningDebugText;
 };
 
 inline constexpr uint16_t kDefaultFailsafeTimeout = 60;
@@ -842,15 +864,46 @@ struct AttestationErrorInfo
 struct CommissioningErrorInfo
 {
     CommissioningErrorInfo(app::Clusters::GeneralCommissioning::CommissioningErrorEnum result) : commissioningError(result) {}
+    CommissioningErrorInfo(app::Clusters::GeneralCommissioning::CommissioningErrorEnum result, CharSpan text) :
+        commissioningError(result), debugText(CharSpanToStdString(text))
+    {}
     app::Clusters::GeneralCommissioning::CommissioningErrorEnum commissioningError;
+    /// Owned copy of the device-supplied `debugText` from
+    /// ArmFailSafeResponse / SetRegulatoryConfigResponse / CommissioningCompleteResponse.
+    /// Empty when the device did not provide debug text.
+    std::string debugText;
+};
+
+struct OperationalCertErrorInfo
+{
+    OperationalCertErrorInfo(app::Clusters::OperationalCredentials::NodeOperationalCertStatusEnum result) :
+        operationalCertStatus(result)
+    {}
+    app::Clusters::OperationalCredentials::NodeOperationalCertStatusEnum operationalCertStatus;
 };
 
 struct NetworkCommissioningStatusInfo
 {
     NetworkCommissioningStatusInfo(app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum result) :
         networkCommissioningStatus(result)
     {}
+    NetworkCommissioningStatusInfo(app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum result, CharSpan text) :
+        networkCommissioningStatus(result), debugText(CharSpanToStdString(text))
+    {}
+    NetworkCommissioningStatusInfo(app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum result, CharSpan text,
+                                   Optional<int32_t> errorValue) :
+        networkCommissioningStatus(result),
+        debugText(CharSpanToStdString(text)), connectNetworkErrorValue(errorValue)
+    {}
     app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum networkCommissioningStatus;
+    /// Owned copy of the device-supplied `debugText` from
+    /// `NetworkConfigResponse` / `ConnectNetworkResponse`. Empty when the device did not
+    /// provide debug text.
+    std::string debugText;
+    /// Optional device-specific connect failure code from ConnectNetworkResponse.errorValue.
+    /// Only populated for ConnectNetwork failures; null for NetworkConfig failures (which
+    /// don't carry an errorValue field).
+    Optional<int32_t> connectNetworkErrorValue;
 };
 
 class CommissioningDelegate
@@ -874,7 +927,7 @@ class CommissioningDelegate
      * kSendOpCertSigningRequest: CSRResponse
      * kGenerateNOCChain: NocChain
      * kSendTrustedRootCert: None
-     * kSendNOC: None
+     * kSendNOC: OperationalCertErrorInfo if AddNOC returned a non-success NodeOperationalCertStatusEnum
      * kConfigureTrustedTimeSource: None
      * kWiFiNetworkSetup: NetworkCommissioningStatusInfo if there is an error
      * kThreadNetworkSetup: NetworkCommissioningStatusInfo if there is an error
@@ -889,8 +942,8 @@ class CommissioningDelegate
      */
     struct CommissioningReport
         : Variant<RequestedCertificate, AttestationResponse, CSRResponse, NocChain, OperationalNodeFoundData, ReadCommissioningInfo,
-                  AttestationErrorInfo, CommissioningErrorInfo, NetworkCommissioningStatusInfo, TimeZoneResponseInfo,
-                  Credentials::JCM::TrustVerificationError>
+                  AttestationErrorInfo, CommissioningErrorInfo, OperationalCertErrorInfo, NetworkCommissioningStatusInfo,
+                  TimeZoneResponseInfo, Credentials::JCM::TrustVerificationError>
     {
         CommissioningReport() : stageCompleted(CommissioningStage::kError) {}
         CommissioningStage stageCompleted;

src/controller/DevicePairingDelegate.h

@@ -103,6 +103,33 @@ class DLL_EXPORT DevicePairingDelegate
                                         Optional<Credentials::AttestationVerificationResult> additionalErrorInfo)
     {}
 
+    /**
+     * @brief
+     *   Called when commissioning fails, with the full structured CompletionStatus from the
+     *   commissioning state machine. Provides access to detailed status that the 4-arg overload
+     *   above drops on the floor.
+     *
+     *   Default implementation forwards to the legacy 4-arg overload above, so existing
+     *   delegates keep working unchanged. The legacy form only carries `err`, `failedStage`,
+     *   and `attestationResult` — the following CompletionStatus fields are silently dropped
+     *   when the default forwarder runs:
+     *
+     *     - commissioningError                (GeneralCommissioning cluster CommissioningErrorEnum)
+     *     - networkCommissioningStatus + connectNetworkErrorValue
+     *     - operationalCertStatus             (OperationalCredentials NOCResponse status)
+     *     - commissioningDebugText            (device-supplied text from ArmFailSafe /
+     *                                          SetRegulatoryConfig / CommissioningComplete)
+     *     - networkCommissioningDebugText     (device-supplied text from NetworkConfig /
+     *                                          ConnectNetwork)
+     *
+     *   Override this overload (instead of the 4-arg one) to receive those fields.
+     */
+    virtual void OnCommissioningFailure(PeerId peerId, const CompletionStatus & completionStatus)
+    {
+        OnCommissioningFailure(peerId, completionStatus.err, completionStatus.failedStage.ValueOr(CommissioningStage::kError),
+                               completionStatus.attestationResult);
+    }
+
     virtual void OnCommissioningStatusUpdate(PeerId peerId, CommissioningStage stageCompleted, CHIP_ERROR error) {}
 
     /**