sessiond: restore the exact brightness the dim captured, not a guessed floor

src/sessiond/device_state.rs

@@ -948,6 +948,21 @@ pub struct DeviceStateMachine {
     /// deadline after which the panel goes dark regardless — see
     /// `LOCK_ACK_BUDGET` for why that fail-open is the documented choice.
     pending_blank: Option<Instant>,
+    /// The exact panel brightness captured immediately before dimming.
+    ///
+    /// The dim used to be `brightnessctl -s set 10` and the restore a
+    /// save/restore pair with a floor: anything that came back under 26/255
+    /// was pushed to 40%. That floor was defending against the old hypridle
+    /// `-s`/`-r` wedge — a second save while already dim pinned brightness at
+    /// 10 and every later wake looked like dead glass — but sessiond's
+    /// `dimmed` field already makes a double save impossible, so the floor was
+    /// guarding a bug that no longer exists and corrupting a real setting to
+    /// do it. A phone deliberately run dark came back *brighter* than it
+    /// started, which is the jump on tap-to-dismiss.
+    ///
+    /// The machine remembers the number instead of inferring it. `None` means
+    /// the capture failed, and only then is the floor the right answer.
+    brightness_before_dim: Option<u32>,
     /// The last raw proximity reading, before debounce. `sensor_evidence`
     /// carries the believed value; this carries what the sensor actually said.
     proximity_raw: bool,
@@ -1019,6 +1034,7 @@ impl DeviceStateMachine {
             blank_requested: false,
             dimmed: false,
             pending_blank: None,
+            brightness_before_dim: None,
             proximity_raw: false,
             proximity_since: None,
             // Loaded, not defaulted: the user's Auto-Lock choices are settings,
@@ -1491,6 +1507,19 @@ impl DeviceStateMachine {
         self.state
     }
 
+    /// Record the brightness the panel was at when the dim fired. The executor
+    /// reads the hardware; the machine is what remembers.
+    pub fn note_brightness_before_dim(&mut self, value: Option<u32>) {
+        self.brightness_before_dim = value;
+    }
+
+    /// The captured pre-dim brightness, consumed by the restore. Taken rather
+    /// than read: a stale value surviving into the next dim cycle is exactly
+    /// the class of bug this replaced.
+    pub fn take_brightness_before_dim(&mut self) -> Option<u32> {
+        self.brightness_before_dim.take()
+    }
+
     /// Should this wake be refused because something is over the sensor?
     ///
     /// Tap-to-wake only. A double tap is the one wake source a pocket can
@@ -2554,6 +2583,38 @@ mod tests {
         );
     }
 
+    #[test]
+    fn the_pre_dim_brightness_is_remembered_and_consumed_once() {
+        // Casey's report: the panel dims, and a tap to dismiss brings it back
+        // at a different level. Cause was `blueline-undim`'s floor — anything
+        // restoring under 26/255 was pushed to 40%, so a phone deliberately
+        // run dark came back brighter than it started. The machine remembers
+        // the number now instead of the executor guessing at it.
+        let mut sm = DeviceStateMachine::new();
+        assert_eq!(sm.take_brightness_before_dim(), None, "nothing captured yet");
+
+        sm.note_brightness_before_dim(Some(18));
+        assert_eq!(
+            sm.take_brightness_before_dim(),
+            Some(18),
+            "a value below the old floor must survive the round trip unchanged"
+        );
+        assert_eq!(
+            sm.take_brightness_before_dim(),
+            None,
+            "taken, not read — a stale capture must not survive into the next dim"
+        );
+    }
+
+    #[test]
+    fn a_failed_brightness_read_falls_through_to_the_floor() {
+        // The one case the floor is right for: nothing knows what the panel
+        // was at, so coming back dark is worse than coming back wrong.
+        let mut sm = DeviceStateMachine::new();
+        sm.note_brightness_before_dim(None);
+        assert_eq!(sm.take_brightness_before_dim(), None);
+    }
+
     #[test]
     fn only_tap_to_wake_is_vetoed_by_a_covered_sensor() {
         // A double tap is the one wake a pocket can produce by itself. The

src/sessiond/server.rs

@@ -211,43 +211,50 @@ fn execute(shared: &Arc<Shared>, action: Action) {
         return;
     }
 
+    // Dim and restore carry a number, so they are not table entries. The
+    // machine remembers the exact brightness the panel was at; the executor
+    // reads the hardware and sets it back. Neither `brightnessctl -s/-r` nor
+    // the floor in `blueline-undim` can tell a mistakenly-saved dim value from
+    // a phone the user deliberately runs dark — that guess is what made a
+    // tap-to-dismiss come back at a different level than it started.
+    if action == Action::Dim {
+        let before = read_brightness();
+        if before.is_none() {
+            warn!("could not read brightness before dimming — restore will use the floor");
+        }
+        shared
+            .lock()
+            .device_state
+            .note_brightness_before_dim(before);
+    }
+    if action == Action::Restore {
+        if let Some(value) = shared.lock().device_state.take_brightness_before_dim() {
+            run_executor(
+                shared,
+                "brightnessctl",
+                &["set", &value.to_string()],
+                "panel-restore",
+                action,
+            );
+            return;
+        }
+        // No capture. This is the only case the floor is right for: the panel
+        // may be sitting at the dim level with nothing that knows better.
+        run_executor(shared, UNDIM_EXECUTOR, &[], "panel-restore", action);
+        return;
+    }
+
     let (program, args, label): (&str, Vec<&str>, &str) = match action {
-        // Save-then-dim. sessiond guarantees this runs once per dim, which is
+        // sessiond guarantees this runs once per dim, which is what the old
-        // what the old hypridle `-s/-r` listener could not: a second save
+        // hypridle `-s`/`-r` listener could not: a second save while already
-        // while already dim is what pinned brightness at 10/255.
+        // dim is what pinned brightness at 10/255.
-        Action::Dim => ("brightnessctl", vec!["-s", "set", "10"], "panel-dim"),
+        Action::Dim => ("brightnessctl", vec!["set", "10"], "panel-dim"),
-        Action::Restore => (UNDIM_EXECUTOR, vec![], "panel-restore"),
+        Action::Restore => unreachable!("handled above; the restore carries a value"),
         Action::Blank => (DPMS_EXECUTOR, vec!["off"], "panel-off"),
         Action::Lock => unreachable!("handled above; the lock is not a shell command"),
     };
 
-    match std::process::Command::new(program).args(&args).status() {
+    run_executor(shared, program, &args, label, action);
-        Ok(status) if status.success() => {
-            if action == Action::Blank {
-                // Record the panel we actually turned off, so the machine's
-                // field tracks the hardware rather than its own intent.
-                let follow_up = shared.lock().device_state.set_panel(false);
-                for a in follow_up {
-                    execute(shared, a);
-                }
-            }
-        }
-        Ok(status) => {
-            let code = status.code().unwrap_or(-1).to_string();
-            warn!("{program} {args:?} exited {code}");
-            shared
-                .lock()
-                .device_state
-                .record_error("device-state", label, &code);
-        }
-        Err(e) => {
-            warn!("could not run {program}: {e}");
-            shared
-                .lock()
-                .device_state
-                .record_error("device-state", label, &e.to_string());
-        }
-    }
 }
 
 /// Act on the machine's decision that the session must lock before the panel
@@ -418,6 +425,58 @@ fn start_ack_timer(shared: &Arc<Shared>, gen: u64) {
 /// agent composing verbs needs to know *which kind* of no it got — not now,
 /// never, or wrong arguments are three different next moves and a sentence
 /// does not distinguish them.
+/// The panel's current brightness, or `None` if it cannot be read.
+///
+/// Read rather than remembered from the last set: the user, the shell, or an
+/// agent may have changed it since, and restoring a value the machine assumed
+/// is how the panel ends up somewhere nobody asked for.
+fn read_brightness() -> Option<u32> {
+    let out = std::process::Command::new("brightnessctl")
+        .arg("get")
+        .output()
+        .ok()?;
+    if !out.status.success() {
+        return None;
+    }
+    String::from_utf8(out.stdout).ok()?.trim().parse().ok()
+}
+
+/// Run one executor and record what it did. Shared by the paths that build
+/// their arguments dynamically and the table below.
+fn run_executor(
+    shared: &Arc<Shared>,
+    program: &str,
+    args: &[&str],
+    label: &str,
+    action: Action,
+) {
+    match std::process::Command::new(program).args(args).status() {
+        Ok(status) if status.success() => {
+            if action == Action::Blank {
+                let follow_up = shared.lock().device_state.set_panel(false);
+                for a in follow_up {
+                    execute(shared, a);
+                }
+            }
+        }
+        Ok(status) => {
+            let code = status.code().unwrap_or(-1).to_string();
+            warn!("{program} {args:?} exited {code}");
+            shared
+                .lock()
+                .device_state
+                .record_error("device-state", label, &code);
+        }
+        Err(e) => {
+            warn!("could not run {program}: {e}");
+            shared
+                .lock()
+                .device_state
+                .record_error("device-state", label, &e.to_string());
+        }
+    }
+}
+
 fn refuse(code: RefusalCode, reason: &str) -> serde_json::Value {
     serde_json::json!({ "ok": false, "code": code.as_str(), "reason": reason })
 }