fix(linux): stop reporting a window with no pid as a failed X11 query (#1513)

Author: y3owk1n

docs/CROSS_PLATFORM.md

@@ -398,7 +398,10 @@ as the bundle identifier for per-app config — but not its PID, because a Wayla
 client cannot read another client's process credentials.
 `SystemPort.FocusedApplicationPID` best-effort matches the app_id against
 `/proc`; with no match it returns `CodeNotSupported` carrying the app_id rather
-than a fabricated number.
+than a fabricated number. A session where *nothing* is focused is a different
+answer from that one and says so: the foreign-toplevel manager answered, and
+`neru doctor` explains it the way the X11 arm's unfocused desktop is explained
+below rather than as an unavailable capability.
 
 **An unfocused desktop is not a failure on X11 either.** The X11 arm of the same
 method reads `_NET_ACTIVE_WINDOW`, which has four ways of not giving you a
@@ -413,6 +416,23 @@ it was. `neru doctor` downgrades the `process` capability to
 now; the `Focused app:` line beside it is what separates "focus a window" from
 "install or fix something".
 
+**And neither is a window that publishes no pid.** One property further down,
+`_NET_WM_PID` splits the same way: a window that is alive and simply does not
+advertise a pid — EWMH makes the property a convention, older toolkits omit it,
+and a client on another machine has none this one could use — is
+`CodeNotSupported` with its own explanation, while a window that closed under
+the query, a failed read and a malformed property are `CodeActionFailed`. The
+two "not supported" answers are separate sentences on purpose: focusing another
+window fixes one and cannot fix the other.
+
+**`FocusedWindowBounds` on X11 tells the same two apart.** The X11 arm reads the
+geometry of whatever `_NET_ACTIVE_WINDOW` names. Nothing focused is `found=false`
+with no error — the caller widens to the active screen and is obeying an answer.
+A display with no live window manager, a failed or malformed property, or a
+window the X server would not describe is an error, so a caller that widens the
+same way can tell it is guessing. That is what the Wayland arms report under
+[Accessibility And Hints](#accessibility-and-hints), on the X11 path.
+
 **App watcher.** macOS gets focus changes pushed from an NSWorkspace observer.
 Linux has no equivalent single API, so `appwatcher/platform_linux.go` subscribes
 to a backend focus-change fd (`linux.SubscribeFocusedApp`: X11 event fd, or the

internal/adapter/accessibility/native/linux/element_x11_cgo.go

@@ -69,10 +69,17 @@ func linuxFocusedApplicationIdentity() (string, int) {
 		C.free(unsafe.Pointer(className))
 	}
 
-	var ok C.int
-	pid := int(C.neru_x11_get_window_pid(display, window, &ok))
+	// The pid answers collapse here the same way the active-window ones do: this
+	// signature has no error to carry, and a window that advertises no pid and
+	// one that closed under the query both leave this caller with no pid to
+	// report. system_x11_window_pid.go is where the difference is told, for the
+	// caller that can act on it.
+	var pid C.ulong
+	if C.neru_x11_get_window_pid(display, window, &pid) != C.int(C.NERU_X11_WINDOW_PID_OK) {
+		pid = 0
+	}
 
-	return bundleID, pid
+	return bundleID, int(pid)
 }
 
 func linuxApplicationBundleIdentifier(pid int) string {

internal/adapter/platform/linux/AGENTS.md

@@ -3,7 +3,7 @@
 Read `../AGENTS.md` first (slots, factory, stubs, generated `wlr_protocol/`). This file covers what it omits. These rules are scattered across file-head comments; violating them fails at link or run time, not compile time.
 
 - **Backend selection is runtime, not build-tag**: KDE and wlroots are identical at compile time (`linux && wayland`); dispatch on `platform.DetectLinuxBackend`, never add a compositor build tag.
-- **Every cgo symbol has a `_nocgo.go` twin** returning `derrors.CodeNotSupported` ("requires CGO-enabled Linux builds"); pinned by `system_stub_contract_test.go` — a supported capability must never answer `CodeNotSupported` or return nil. The one exception, on both backends, is a query that worked and found nothing: `FocusedApplicationPID` reports `CodeNotSupported` when no window is focused, so callers degrade instead of surfacing a live desktop as a failure. It is not a gap, and the capability is live-probed rather than declared, so the guardrail sees the two move together.
+- **Every cgo symbol has a `_nocgo.go` twin** returning `derrors.CodeNotSupported` ("requires CGO-enabled Linux builds"); pinned by `system_stub_contract_test.go` — a supported capability must never answer `CodeNotSupported` or return nil. The exceptions, on both backends, are the queries that worked and found nothing: `FocusedApplicationPID` reports `CodeNotSupported` when no window is focused, and again when the window that is focused publishes no pid (`_NET_WM_PID` is a convention EWMH does not require), so callers degrade instead of surfacing a live desktop as a failure. Each wears its own sentinel — `errNoFocusedWindow`, `errNoWindowPID` — because `neru doctor` explains them in different words, and a failure must wear neither. They are not gaps, and the capability is live-probed rather than declared, so the guardrail sees the two move together.
 - **The `.c` files compile in exactly one unit** (`cgo.go`). Packages that call bridge symbols must blank-import `platform/linux` — and `wlr_protocol` separately — or the linker fails with undefined symbols.
 - **Xlib protocol errors go through the one shared trap** (`x11_error_trap.h`), never a second `XSetErrorHandler`. The default handler calls `exit()`, so any request that can legitimately fail — reading pixels off a drawable that shrank, reading a property off a window whose client just died — has to be trapped; and the handler is process-global, so two traps under two mutexes would interleave and leave one caller's requests running under the handler that exits. Keep trapped sections short: everything else that traps waits behind them.
 - **`doc.go` is tagged `linux`, not `linux && cgo` — deliberately.** The `.c` files already gate cgo builds; the broader tag keeps analysis working on other hosts. Don't "fix" it.

internal/adapter/platform/linux/system_common.go

@@ -680,6 +680,16 @@ func (s *SystemAdapter) unavailableDetail(feature string, cause error) string {
 			": the query works and answers as soon as a window takes focus"
 	}
 
+	// The same argument one property down, and the reason it is a second
+	// sentinel rather than the first: a window is focused, and it advertises no
+	// _NET_WM_PID. Nothing is broken and nothing can be installed to change it —
+	// EWMH does not require the property — so the sentence above would send the
+	// user to focus a window they already focused.
+	if errors.Is(cause, errNoWindowPID) {
+		return feature + " found no _NET_WM_PID on the focused window on linux backend " +
+			s.backendLabel() + ": the query works and answers for a window that publishes one"
+	}
+
 	if !nativeBackendsCompiledIn {
 		return feature + " is unavailable: this binary was built without CGO, so the X11 " +
 			"and wlroots client stacks are absent; use a CGO-enabled build"

internal/adapter/platform/linux/system_focused_pid.go

@@ -23,14 +23,15 @@ import (
 func waylandFocusedApplicationPID() (int, error) {
 	appID, ok := WaylandFocusedAppID()
 	if !ok || appID == "" {
-		return 0, derrors.New(
-			derrors.CodeNotSupported,
-			"FocusedApplicationPID: no focused app_id available on this Wayland compositor",
-		)
+		return 0, waylandNoFocusedAppError()
 	}
 
 	pid, found := resolvePIDByAppID(appID, "/proc")
 	if !found {
+		// Not the unfocused-desktop sentinel: a window *is* focused, and the
+		// heuristic that maps its app_id onto a process missed. Wearing the
+		// sentinel here would tell the user to focus a window they already have
+		// focused.
 		return 0, derrors.Newf(
 			derrors.CodeNotSupported,
 			"FocusedApplicationPID: Wayland exposes no PID for the focused app (app_id=%q); no /proc match found",
@@ -41,6 +42,35 @@ func waylandFocusedApplicationPID() (int, error) {
 	return pid, nil
 }
 
+// waylandNoFocusedAppError explains an empty answer from the foreign-toplevel
+// manager, which is the wlroots family's version of an unfocused desktop.
+//
+// It wraps errNoFocusedWindow so the capability surface explains it the way the
+// X11 arm's is explained (#1495): the query works, and it answers as soon as a
+// window takes focus. The CGO-off build takes the other branch, because there
+// the manager is absent rather than idle, and the reassuring sentence would be a
+// lie on a binary that can never answer.
+//
+// What it cannot yet separate is a compositor whose foreign-toplevel manager
+// never bound: WaylandFocusedAppID reports that with the same bare false as an
+// idle session, so on a CGO build both arrive here. Telling them apart needs the
+// wlroots client to say which happened, in both its cgo and nocgo halves.
+func waylandNoFocusedAppError() error {
+	if !nativeBackendsCompiledIn {
+		return derrors.New(
+			derrors.CodeNotSupported,
+			"FocusedApplicationPID: this binary was built without CGO, so the "+
+				"foreign-toplevel client that reports the focused app_id is absent",
+		)
+	}
+
+	return derrors.Wrap(
+		errNoFocusedWindow,
+		derrors.CodeNotSupported,
+		"FocusedApplicationPID: no focused app_id available on this Wayland compositor",
+	)
+}
+
 // resolvePIDByAppID scans procRoot (normally "/proc") for a process whose
 // identity matches appID and returns its PID. It is a heuristic: Wayland
 // app_ids and process names do not always agree, so a miss is expected and

internal/adapter/platform/linux/system_focused_pid_test.go

@@ -3,10 +3,14 @@
 package linux
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
+
+	"github.com/y3owk1n/neru/internal/derrors"
 )
 
 const (
@@ -198,3 +202,47 @@ func TestResolvePIDByAppID(t *testing.T) {
 		}
 	})
 }
+
+// TestWaylandNoFocusedAppError is the wlroots half of what #1495 fixed for X11.
+// The blessed Wayland stack answers an unfocused session the same way an
+// unfocused X11 display does, so `neru doctor` has to explain it the same way:
+// "focused-app inspection is unavailable" on a session where nothing has taken
+// focus yet sends the user looking for something to install.
+func TestWaylandNoFocusedAppError(t *testing.T) {
+	t.Parallel()
+
+	err := waylandNoFocusedAppError()
+
+	if !derrors.IsNotSupported(err) {
+		t.Fatalf("waylandNoFocusedAppError() = %v, want %q so callers degrade through it",
+			err, derrors.CodeNotSupported)
+	}
+
+	// The sentinel's premise is that a native backend answered, which a build
+	// without the foreign-toplevel client cannot have done.
+	wrapsSentinel := errors.Is(err, errNoFocusedWindow)
+	if wrapsSentinel != nativeBackendsCompiledIn {
+		t.Fatalf("waylandNoFocusedAppError() wraps errNoFocusedWindow = %v, want %v",
+			wrapsSentinel, nativeBackendsCompiledIn)
+	}
+
+	if !nativeBackendsCompiledIn {
+		return
+	}
+
+	feature := focusedAppFeature
+	detail := NewSystemAdapter(backendWaylandWlroots).unavailableDetail(feature, err)
+
+	if strings.Contains(detail, "unavailable") {
+		t.Errorf("an unfocused wlroots session is described as %q; it must not claim "+
+			"the capability is unavailable", detail)
+	}
+
+	// "takes focus" rather than "focus": the sentence has to promise the answer
+	// arrives when a window is focused, not merely mention the word.
+	for _, word := range []string{feature, backendWaylandWlroots, "takes focus"} {
+		if !strings.Contains(detail, word) {
+			t.Errorf("the unfocused-session detail %q does not mention %q", detail, word)
+		}
+	}
+}

internal/adapter/platform/linux/system_x11_active_window.go

@@ -49,9 +49,11 @@ const (
 // explains it differently from a failure, because focusing any window fixes it
 // and installing something does not.
 //
-// Only the X11 arm wraps it today. The Wayland arm reports its own
-// CodeNotSupported for the same state without the sentinel, so `neru doctor`
-// still explains that one as an unavailable capability.
+// Both arms wrap it: the X11 one for a display whose _NET_ACTIVE_WINDOW reads
+// None, and the wlroots one for a foreign-toplevel manager with nothing
+// activated (waylandNoFocusedAppError). What it does not cover is a window that
+// has focus and no pid to report — that is errNoWindowPID, a sibling sentinel,
+// because the sentence this one buys would be false there.
 var errNoFocusedWindow = errors.New("no window has focus")
 
 // x11ActiveWindowQueryError returns the error a caller owes for result, or nil

internal/adapter/platform/linux/system_x11_active_window_test.go

@@ -19,6 +19,10 @@ const activeWindowProperty = "_NET_ACTIVE_WINDOW"
 // manager from one that exited leaving its advertisements on the root window.
 const supportingWMCheckProperty = "_NET_SUPPORTING_WM_CHECK"
 
+// focusedAppFeature is the capability label `neru doctor` prints these details
+// under, shared by every test that reads one back.
+const focusedAppFeature = "focused-app inspection"
+
 // TestX11ActiveWindowQueryError is the whole point of splitting the
 // _NET_ACTIVE_WINDOW answers apart: a desktop with nothing focused is a state
 // callers degrade through, and the three ways the query can fail are failures
@@ -160,7 +164,7 @@ func TestX11ActiveWindowQueryError_OnlyTheUnfocusedDesktopCarriesTheSentinel(t *
 // what has to stop claiming the subsystem is gone.
 func TestSystemAdapter_UnavailableDetail_SeparatesAnUnfocusedDesktopFromAFailure(t *testing.T) {
 	adapter := NewSystemAdapter(backendX11)
-	feature := "focused-app inspection"
+	feature := focusedAppFeature
 
 	unfocused := adapter.unavailableDetail(feature, x11ActiveWindowQueryError(x11ActiveWindowNone))
 

internal/adapter/platform/linux/system_x11_cgo.go

@@ -122,22 +122,39 @@ func x11FocusedApplicationPID() (int, error) {
 		return 0, queryErr
 	}
 
-	var ok C.int
-	pid := C.neru_x11_get_window_pid(display, window, &ok)
-	if ok == 0 {
-		// Either the window exposes no pid, or _NET_ACTIVE_WINDOW named one
-		// that has since closed — a window manager that exited leaves the
-		// property behind pointing at a window nobody updates any more.
-		return 0, derrors.New(
-			derrors.CodeActionFailed,
-			"failed to query _NET_WM_PID for the active X11 window; it exposes no pid, "+
-				"or the window it names has closed",
-		)
+	var pid C.ulong
+
+	pidErr := x11WindowPIDError(x11WindowPIDQuery(display, window, &pid))
+	if pidErr != nil {
+		return 0, pidErr
 	}
 
 	return int(pid), nil
 }
 
+// x11WindowPIDQuery reads _NET_WM_PID off window and translates the C result
+// code into this package's vocabulary. Like x11ActiveWindowQuery, the mapping is
+// by name rather than by value, so the two numberings are free to differ.
+func x11WindowPIDQuery(display *C.Display, window C.Window, pid *C.ulong) x11WindowPIDResult {
+	switch C.neru_x11_get_window_pid(display, window, pid) {
+	case C.int(C.NERU_X11_WINDOW_PID_OK):
+		return x11WindowPIDFound
+	case C.int(C.NERU_X11_WINDOW_PID_ABSENT):
+		return x11WindowPIDAbsent
+	case C.int(C.NERU_X11_WINDOW_PID_WINDOW_GONE):
+		return x11WindowPIDWindowGone
+	case C.int(C.NERU_X11_WINDOW_PID_QUERY_FAILED):
+		return x11WindowPIDQueryFailed
+	case C.int(C.NERU_X11_WINDOW_PID_MALFORMED):
+		return x11WindowPIDMalformed
+	default:
+		// A result the header grew and this switch has not. Reporting it as a
+		// failed query is the safe direction: the caller surfaces it instead of
+		// degrading past a state nobody has classified.
+		return x11WindowPIDQueryFailed
+	}
+}
+
 // x11FocusedAppID returns the WM_CLASS "class" of the active X11 window, used
 // as the per-app bundle identifier (matching accessibility's focused-app
 // identity on X11). The bool is false when DISPLAY is unset, no window is
@@ -236,24 +253,36 @@ func x11ScreenEventFD() (int, bool) {
 }
 
 // x11FocusedWindowBounds returns the global bounds of the currently focused
-// window via _NET_ACTIVE_WINDOW. found is false (with a nil error) when there is
-// no active window or its geometry could not be queried, so callers fall back to
-// the active-screen bounds.
+// window via _NET_ACTIVE_WINDOW.
+//
+// found is false with a nil error only when the display answered that nothing
+// has focus; callers widen to the active-screen bounds and are obeying an
+// answer. A query that never got one — no live window manager, a failed or
+// malformed property, or a window the X server would not describe — is an error,
+// so the same widening can be told apart from guessing.
 func x11FocusedWindowBounds() (image.Rectangle, bool, error) {
 	display, err := x11OpenDisplay()
 	if err != nil {
 		return image.Rectangle{}, false, err
 	}
 	defer C.neru_x11_close_display(display)
 
-	var posX, posY, width, height C.int
-	found := C.neru_x11_get_focused_window_bounds(
-		display, &posX, &posY, &width, &height,
-	)
-	if found == 0 {
+	window, result := x11ActiveWindowQuery(display)
+
+	readGeometry, queryErr := x11ShouldReadWindowGeometry(result)
+	if queryErr != nil {
+		return image.Rectangle{}, false, queryErr
+	}
+
+	if !readGeometry {
 		return image.Rectangle{}, false, nil
 	}
 
+	var posX, posY, width, height C.int
+	if C.neru_x11_get_window_bounds(display, window, &posX, &posY, &width, &height) == 0 {
+		return image.Rectangle{}, false, x11WindowGeometryError()
+	}
+
 	return image.Rect(
 		int(posX),
 		int(posY),

internal/adapter/platform/linux/system_x11_integration_linux_test.go

@@ -47,11 +47,11 @@ func TestX11FocusedApplicationPID_AWindowManagerIsNeverMistakenForNoWindowManage
 	case err == nil:
 		// A window is focused and exposes its PID — the ordinary path.
 	case derrors.IsNotSupported(err):
-		// Nothing is focused. That is the answer this ticket exists to allow,
-		// and callers degrade through it.
-	case strings.Contains(err.Error(), windowPIDProperty):
-		// The focused window exposes no _NET_WM_PID. A different property, and
-		// not what this test is about.
+		// Nothing is focused, or the focused window advertises no pid. Both are
+		// answers this ticket exists to allow, and callers degrade through them.
+	case strings.Contains(err.Error(), wmPIDProperty):
+		// The pid property failed to read. A different property, and not what
+		// this test is about.
 	default:
 		t.Fatalf(
 			"a display whose _NET_SUPPORTING_WM_CHECK handshake completes answered %v "+
@@ -61,10 +61,6 @@ func TestX11FocusedApplicationPID_AWindowManagerIsNeverMistakenForNoWindowManage
 	}
 }
 
-// windowPIDProperty names the second property the focused-app path reads, so
-// its failure can be told apart from the active-window query's.
-const windowPIDProperty = "_NET_WM_PID"
-
 // supportingWMCheck is the property whose two-step self-reference proves a
 // window manager is running now rather than having run once.
 const supportingWMCheck = "_NET_SUPPORTING_WM_CHECK"