Fix the nested guard: it answered wrong for a real session, and missed a case

The guard added earlier tonight asked "is WAYLAND_DISPLAY or DISPLAY set".
Both Wayland backends call set_var("WAYLAND_DISPLAY", ...) on themselves the
moment they bind their own socket, so after startup that question answers
"yes" for a real udev session too. Every publish on the config-reload path
runs after that point, which means a live session that reloaded its config
would have stopped publishing its own GTK settings - the exact opposite of
what the guard is for. It also treated srdwm-x as nested, because an X11
session naturally has DISPLAY set, even though srdwm is that display's
window manager and not a client of anything.

Nestedness is now read once, at startup, before any backend is up, and
only the Wayland backend can be nested. A test pins the second half.

The same reasoning applies to window_memory::save_all, which had no guard
at all: a nested instance shares HOME with the session it runs inside, so
dragging a test window would overwrite where that application opens in the
real session - a 1280x800 test window's position applied to a 3840x1080
desktop. Loading stays unconditional and deliberate: honouring what a real
session remembered is right, writing back over it is not. That side reads
srdwm_wayland::running_nested, recorded by connect at the moment it picks
the winit backend, since the environment can no longer be asked afterward.

Verified: a nested run with a scratch config left both the stylesheet and
window-memory.json untouched (md5 before and after, and no test window's
app-id in the store).

3 files changed, 90 insertions, 25 deletions

diff --git a/crates/srdwm/src/main.rs b/crates/srdwm/src/main.rs
index 6f4cf88..5e5d5c5 100644
--- a/crates/srdwm/src/main.rs
+++ b/crates/srdwm/src/main.rs
@@ -126,24 +126,33 @@ const CONFIG_POLL_INTERVAL: std::time::Duration = std::time::Duration::from_secs
 /// not an error worth interrupting startup for. The compositor's own
 /// titlebars are already correct either way; this only brings the
 /// self-decorating clients into line.
-/// True when this srdwm is running nested inside another compositor's
-/// session rather than owning the machine's own.
-///
-/// Same test `srdwm_wayland::connect` uses to pick the winit backend over
-/// udev/DRM: a host `WAYLAND_DISPLAY` or `DISPLAY` means there is already a
-/// session, and this process is a window inside it.
+/// True when this srdwm runs nested inside another compositor's session --
+/// a window on someone else's desktop - rather than owning the machine's
+/// own outputs.
 ///
 /// Anything that writes to the *user's own desktop configuration* must be
-/// gated on this. A nested instance is a test or development window; it is
-/// not the shell, and it has no business rewriting the settings the real
-/// session is using. Learned the hard way: nested runs started with a
-/// scratch srdwm config, which naturally does not set `button_style`, took
-/// the built-in default and wrote traffic-light CSS straight into the real
+/// gated on this. A nested instance is a test or development window: it
+/// shares `HOME` with the real session, but it is not the shell, and it has
+/// no business rewriting the settings the real session is using. Learned
+/// the hard way: nested runs started with a scratch srdwm config, which
+/// naturally does not set `button_style`, took the built-in default and
+/// wrote traffic-light CSS straight into the real
 /// `~/.config/gtk-3.0/srdwm-buttons.css` - silently changing the look of
 /// every GTK app in the actual session, from a throwaway compositor that
 /// was testing something unrelated.
-fn running_nested() -> bool {
-    std::env::var_os("WAYLAND_DISPLAY").is_some() || std::env::var_os("DISPLAY").is_some()
+///
+/// Two things this must get right, both of which a bare "is WAYLAND_DISPLAY
+/// set" test gets wrong:
+///
+/// - It must be read *before* the backend binds its own socket. Both
+///   Wayland backends call `set_var("WAYLAND_DISPLAY", ...)` on themselves
+///   once they are up, so afterward even a real udev session looks nested.
+///   Call this once at startup and keep the answer.
+/// - Only the Wayland backend can nest. srdwm-x is the window manager of
+///   the X display it runs on, not a client of another compositor, so a set
+///   `DISPLAY` says nothing about it.
+fn running_nested(kind: PlatformKind) -> bool {
+    matches!(kind, PlatformKind::Wayland) && (std::env::var_os("WAYLAND_DISPLAY").is_some() || std::env::var_os("DISPLAY").is_some())
 }
 
 /// The stylesheet srdwm owns and rewrites, named so it is obvious in a
@@ -234,8 +243,8 @@ fn gtk_button_css(traffic_lights: bool) -> String {
 ///
 /// Best-effort: a missing directory or an unwritable file is logged at
 /// debug and skipped. srdwm's own titlebars are already correct regardless.
-fn publish_gtk_stylesheet(wm: &Rc<RefCell<WindowManager>>) {
-    if running_nested() {
+fn publish_gtk_stylesheet(wm: &Rc<RefCell<WindowManager>>, nested: bool) {
+    if nested {
         return;
     }
     let Ok(home) = std::env::var("HOME") else { return };
@@ -267,8 +276,8 @@ fn publish_gtk_stylesheet(wm: &Rc<RefCell<WindowManager>>) {
     }
 }
 
-fn publish_gtk_button_layout(wm: &Rc<RefCell<WindowManager>>) {
-    if running_nested() {
+fn publish_gtk_button_layout(wm: &Rc<RefCell<WindowManager>>, nested: bool) {
+    if nested {
         return;
     }
     let layout = if wm.borrow().theme.buttons_left { "close,minimize,maximize:" } else { ":minimize,maximize,close" };
@@ -959,8 +968,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     // After `apply_general_settings`, which is what copies the config's
     // `button_side` into the theme - publishing before it would broadcast
     // the built-in default rather than the user's choice.
-    publish_gtk_button_layout(&wm);
-    publish_gtk_stylesheet(&wm);
+    // Read once, here, and carried to every later call: after the backend
+    // is up, `running_nested`'s own test can no longer answer honestly.
+    let nested = running_nested(kind);
+    publish_gtk_button_layout(&wm, nested);
+    publish_gtk_stylesheet(&wm, nested);
     apply_default_layout(&engine, &wm);
     let running = engine.running_flag();
     // `general.config_reload_on_write` - on by default. A programmable
@@ -1070,8 +1082,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     }
                     apply_general_settings(&engine, &wm);
                     publish_keybindings(&engine, &wm);
-                    publish_gtk_button_layout(&wm);
-    publish_gtk_stylesheet(&wm);
+                    publish_gtk_button_layout(&wm, nested);
+                    publish_gtk_stylesheet(&wm, nested);
                     // After `apply_general_settings`, which rebuilds the
                     // theme from the config file - see
                     // `WindowManager::live_settings` for why a hand-made
@@ -1096,8 +1108,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             }
             apply_general_settings(&engine, &wm);
             publish_keybindings(&engine, &wm);
-            publish_gtk_button_layout(&wm);
-    publish_gtk_stylesheet(&wm);
+            publish_gtk_button_layout(&wm, nested);
+            publish_gtk_stylesheet(&wm, nested);
             drop_live_settings_the_config_states(&engine, &wm);
             srdwm_platform::replay_live_settings(&wm);
             // After the reload, so a handler edited in the config since
@@ -1117,8 +1129,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                         }
                         apply_general_settings(&engine, &wm);
                         publish_keybindings(&engine, &wm);
-                        publish_gtk_button_layout(&wm);
-    publish_gtk_stylesheet(&wm);
+                        publish_gtk_button_layout(&wm, nested);
+                        publish_gtk_stylesheet(&wm, nested);
                         drop_live_settings_the_config_states(&engine, &wm);
                         srdwm_platform::replay_live_settings(&wm);
                     } else if !engine.dispatch_keybinding(&combo) {
@@ -1231,6 +1243,16 @@ mod tests {
         }
     }
 
+    /// srdwm-x is the window manager of the X display it runs on, not a
+    /// client of another compositor, so a set `DISPLAY` must not make it
+    /// look nested and stop it publishing the user's own settings.
+    #[test]
+    fn only_the_wayland_backend_can_be_nested() {
+        assert!(!running_nested(PlatformKind::X11));
+        assert!(!running_nested(PlatformKind::Windows));
+        assert!(!running_nested(PlatformKind::MacOS));
+    }
+
     /// A `*/` inside a commented-out style would end the comment early and
     /// leave that style's rules live alongside the configured one.
     #[test]
diff --git a/crates/wayland/src/lib.rs b/crates/wayland/src/lib.rs
index e396491..13d863e 100644
--- a/crates/wayland/src/lib.rs
+++ b/crates/wayland/src/lib.rs
@@ -96,6 +96,31 @@ pub(crate) fn err(e: impl std::fmt::Display) -> PlatformError {
 /// nested-vs-native. Falls back to winit if udev initialization fails for
 /// any reason (no seat access, no DRM device, ...), logging why rather than
 /// failing outright.
+static NESTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false);
+
+/// True when this srdwm runs nested inside another compositor's session --
+/// a window on someone else's desktop - rather than owning the machine's
+/// own outputs.
+///
+/// Anything that writes to the *user's own desktop configuration or state*
+/// must be gated on this. A nested instance is a test or development
+/// window: it shares `HOME` with the real session, but it is not the shell,
+/// and it has no business rewriting settings the real session is using.
+/// Learned twice - a nested run rewrote the real
+/// `~/.config/gtk-3.0/srdwm-buttons.css` from its scratch config's
+/// defaults, changing every GTK app's window buttons in the live session;
+/// and `window_memory::save_all` would do the same to where every
+/// application opens.
+///
+/// Recorded by `connect` at the moment the backend is chosen, and read
+/// afterward, because it cannot be re-derived later: both backends set
+/// `WAYLAND_DISPLAY` on themselves once they bind their own socket, so
+/// "is there a WAYLAND_DISPLAY" answers yes for a real udev session too
+/// the moment it is up.
+pub fn running_nested() -> bool {
+    NESTED.load(std::sync::atomic::Ordering::Relaxed)
+}
+
 pub fn connect(wm: Rc<RefCell<WindowManager>>, bound_keys: &[String], repeat_keys: &[String]) -> PlatformResult<Box<dyn Platform>> {
     let no_host_display = std::env::var_os("WAYLAND_DISPLAY").is_none() && std::env::var_os("DISPLAY").is_none();
     if no_host_display {
@@ -104,5 +129,11 @@ pub fn connect(wm: Rc<RefCell<WindowManager>>, bound_keys: &[String], repeat_key
             Err(e) => log::warn!("udev/DRM backend unavailable ({e}); falling back to nested winit backend"),
         }
     }
+    // Only the winit backend is reached from here, and it is reached only
+    // when there is a host session to nest inside - either one was found
+    // above, or udev failed and this is a fallback into someone else's
+    // session. Set before `WaylandPlatform::connect`, which binds a socket
+    // and overwrites `WAYLAND_DISPLAY` with its own.
+    NESTED.store(true, std::sync::atomic::Ordering::Relaxed);
     Ok(Box::new(WaylandPlatform::connect(wm, bound_keys, repeat_keys)?))
 }
diff --git a/crates/wayland/src/window_memory.rs b/crates/wayland/src/window_memory.rs
index b2d3711..4376274 100644
--- a/crates/wayland/src/window_memory.rs
+++ b/crates/wayland/src/window_memory.rs
@@ -113,7 +113,19 @@ pub(crate) fn load() -> HashMap<String, PersistedGeometry> {
 /// story (the same reasoning `monitor_layout::save_output` and `desktop_
 /// icons_state`'s own saver already settled on for the identical shape of
 /// problem).
+///
+/// A nested srdwm never writes it. It shares `HOME` with the session it is
+/// running inside, so a window dragged around in a test compositor would
+/// otherwise overwrite where that same application opens in the user's real
+/// session - a 1280x800 test window's position applied to a 3840x1080
+/// desktop. Loading stays unconditional and deliberate (see `connect`'s own
+/// call site): honouring what a real session remembered is right, writing
+/// back over it is not. Same reasoning as `publish_gtk_stylesheet`'s own
+/// nested guard.
 pub(crate) fn save_all<'a>(entries: impl Iterator<Item = (&'a str, (i32, i32, u32, u32))>) {
+    if crate::running_nested() {
+        return;
+    }
     let apps: HashMap<String, PersistedGeometry> =
         entries.map(|(app_id, (x, y, width, height))| (app_id.to_string(), PersistedGeometry { x, y, width, height })).collect();
     let memory = PersistedWindowMemory { apps };