Broker: a failed calibration is an Error, and ending a run clears what it left

Three things a caller could not see, all in the same lifecycle.

A calibration that FAILS now goes to Error rather than Inactive. The wait
endpoints turn Error into a 500 carrying the message, while Inactive is a
bodiless 502 - so the reason a pedestal or mask sequence failed was thrown away
on the way out. Inactive is also where a deliberate Deactivate() leaves the
broker, which made a failure indistinguishable from a detector someone had
powered down. Initialize() clears Error, so recovery is what it was. The four
CANCELLATION paths keep Inactive: a cancel is not a failure, and c29dd67b5
separated the two deliberately.

Cancel() and Deactivate() clear start_exception. JFJochStateMachine.h says every
entry point that begins new work clears it; these two end work instead, and were
missed. A failed /start followed by /deactivate left every later wait call
rethrowing a failure from a run on a detector that is no longer powered, until
some later /start or /initialize happened to clear it.

Start() and Deactivate() clear scan_result. It is only ever assigned at the end
of a successful measurement, so a run that failed to start left /scan_result
answering with the PREVIOUS run's images, under the new run number and with
nothing to mark them as not its own - the same stale-read Initialize() has always
cleared. Deactivate() clears it for the same reason it clears the rest: the
result describes a measurement on a detector being turned off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016L1qig74oYQzfUJJZbbxFh

broker/JFJochStateMachine.cpp

@@ -120,7 +120,11 @@ void JFJochStateMachine::CalibrateDetector(std::unique_lock<std::mutex> ul) {
     } catch (const std::exception &e) {
         logger.Error("Calibration sequence error {}", e.what());
         // The calibration is in an undefined state, so the detector has to be initialized again.
-        SetState(JFJochState::Inactive, e.what(), BrokerStatus::MessageSeverity::Error);
+        // Error rather than Inactive: /wait_till_done and /wait_until_running answer Error with a 500
+        // carrying this message, while Inactive is a bodiless 502 that loses the reason - and Inactive
+        // is also what a deliberate Deactivate() leaves behind, so a failure would be indistinguishable
+        // from a detector someone powered down. Initialize() clears Error, so recovery is unchanged.
+        SetState(JFJochState::Error, e.what(), BrokerStatus::MessageSeverity::Error);
         c.notify_all(); // ul unlocks on the way out
         throw;
     }
@@ -162,7 +166,7 @@ bool JFJochStateMachine::TakeDarkMaskInternal(std::unique_lock<std::mutex> &ul)
     ul.lock();
 
     if (mask_output.receiver_output.dark_mask_result.size() != local_experiment.GetPixelsNum()) {
-        SetState(JFJochState::Inactive, "Mask not collected properly", BrokerStatus::MessageSeverity::Error);
+        SetState(JFJochState::Error, "Mask not collected properly", BrokerStatus::MessageSeverity::Error);
         return false;
     }
     pixel_mask.LoadDarkBadPixelMask(local_experiment, mask_output.receiver_output.dark_mask_result);
@@ -209,7 +213,7 @@ bool JFJochStateMachine::TakePedestalInternalG0(std::unique_lock<std::mutex> &ul
     ul.lock();
 
     if (!ImportPedestalG0(pedestal_output.receiver_output)) {
-        SetState(JFJochState::Inactive,
+        SetState(JFJochState::Error,
                  "Pedestal not collected properly",
                  BrokerStatus::MessageSeverity::Error);
         return false;
@@ -254,7 +258,7 @@ bool JFJochStateMachine::TakePedestalInternalG1(std::unique_lock<std::mutex> &ul
     ul.lock();
 
     if (!ImportPedestalG1G2(pedestal_output.receiver_output, 1, storage_cell)) {
-        SetState(JFJochState::Inactive,
+        SetState(JFJochState::Error,
                  "Pedestal not collected properly",
                  BrokerStatus::MessageSeverity::Error);
         return false;
@@ -297,7 +301,7 @@ bool JFJochStateMachine::TakePedestalInternalG2(std::unique_lock<std::mutex> &ul
     ul.lock();
 
     if (!ImportPedestalG1G2(pedestal_output.receiver_output, 2, storage_cell)) {
-        SetState(JFJochState::Inactive,
+        SetState(JFJochState::Error,
                  "Pedestal not collected properly",
                  BrokerStatus::MessageSeverity::Error);
         return false;
@@ -387,6 +391,12 @@ void JFJochStateMachine::Start(const DatasetSettings &settings, bool async) {
     // previous run's failure behind for the next wait call to report.
     start_exception = nullptr;
 
+    // The same for the previous run's scan result. It is only ever replaced at the END of a
+    // successful measurement, so a run that fails to start - or is rejected here - would otherwise
+    // leave /scan_result answering with the last run's images, under this run's number and with
+    // nothing to say they are not its own. Initialize() already clears it for the same reason.
+    scan_result = {};
+
     experiment.ImportDatasetSettings(settings);
 
     cancel_sequence = false;
@@ -532,6 +542,9 @@ void JFJochStateMachine::MeasurementThread() {
 void JFJochStateMachine::Cancel() {
     // This is inconsistency in naming - need to solve later
     std::unique_lock ul(m);
+    // Cancelling supersedes a pending start failure: the caller has taken the run in hand, and the
+    // next wait must report what the cancel did, not what the start before it failed to do.
+    start_exception = nullptr;
     if ((state == JFJochState::Calibration) || (state == JFJochState::Measuring)) {
         services.Cancel();
         cancel_sequence = true;
@@ -554,6 +567,16 @@ void JFJochStateMachine::Deactivate() {
     if (IsRunning())
         throw WrongDAQStateException("Cannot deactivate while the detector is busy");
 
+    // Powering down supersedes a pending start failure. Without this it outlives the detector it
+    // belongs to: every later wait call keeps rethrowing a failure from a run on hardware that is no
+    // longer even on, until the next Initialize() or Start() happens to clear it.
+    start_exception = nullptr;
+
+    // The same for the last run's scan result, and for the same reason: it describes a measurement on
+    // a detector that is being turned off, and Initialize() - the only way back from here - clears it
+    // anyway. Leaving it would let /scan_result answer for a session that has ended.
+    scan_result = {};
+
     // Reap the finished thread, but do not let a failure it stored stop the power-off: the state is
     // Error precisely because that run failed, and leaving the detector powered is worse than
     // losing an error message that was already reported when it happened.