From d8fa49be37ecf6885b189f836772007511e31237 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= <spartanj@gmail.com>
Date: Sun, 7 Jun 2026 22:12:28 -0300
Subject: [PATCH] Fix xvfb-run-eepp, update unit-tests.md.

---
 .agent/rules/unit-tests.md     | 20 ++++++++++++++------
 projects/scripts/xvfb-run-eepp | 26 +++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/.agent/rules/unit-tests.md b/.agent/rules/unit-tests.md
index 2d1e50e12..d3c8a7266 100644
--- a/.agent/rules/unit-tests.md
+++ b/.agent/rules/unit-tests.md
@@ -5,15 +5,23 @@ This project relies on a comprehensive suite of unit tests to prevent regression
 ## Running Tests
 The test binary manages its own current working directory, so you can execute it from anywhere.
 
-*   **Standard Execution:**
-    `bin/unit_tests/eepp-unit_tests-debug`
-*   **Linux & FreeBSD Execution (Required for Desktop Environments):**
-    Tests open ~400 individual windows. To prevent disrupting the desktop environment, run them in an isolated framebuffer using `xvfb-run-eepp` (a project-specific fork of `xvfb-run` that enables `--auto-servernum`, sets the default screen to 1280x1024x24, and injects `ASAN_OPTIONS=detect_leaks=0` automatically):
+*   **Default Execution for Agents on Linux & FreeBSD:**
+    Always run unit tests through the project wrapper unless the user explicitly asks for a different harness:
     `projects/scripts/xvfb-run-eepp bin/unit_tests/eepp-unit_tests-debug`
+*   **Why the wrapper is required:**
+    Tests open ~400 individual windows. The wrapper runs them in an isolated framebuffer, enables race-safe automatic display selection for concurrent agent test runs, sets the default screen to `1280x1024x24`, and injects `ASAN_OPTIONS=detect_leaks=0` automatically.
+*   **Do not skip the wrapper for filtered tests:**
+    A focused test still needs the same wrapper:
+    `projects/scripts/xvfb-run-eepp bin/unit_tests/eepp-unit_tests-debug --filter="FontRendering.*Offset*"`
+*   **Fallback only when the wrapper itself fails:**
+    If `projects/scripts/xvfb-run-eepp` fails before launching the test binary, report that wrapper failure and then use this fallback to keep verification moving:
+    `ASAN_OPTIONS=detect_leaks=0 xvfb-run -a -s "-screen 0 1280x1024x24" bin/unit_tests/eepp-unit_tests-debug`
+    Do not use plain `xvfb-run` as the first attempt for GUI/unit tests.
+*   **Direct Execution (Only for non-window tests or explicit user requests):**
+    `bin/unit_tests/eepp-unit_tests-debug`
 *   **Filtering Tests:**
     Use the `--filter` parameter to run specific tests (supports glob patterns).
-    *Example (runs all tests with "Offset" in the name):*
-    `bin/unit_tests/eepp-unit_tests-debug --filter="FontRendering.*Offset*"`
+    Keep the wrapper in front of the binary unless the test is known not to create windows.
 
 ## Writing New Tests
 Writing new tests is highly encouraged, but depends on the context of your changes:
diff --git a/projects/scripts/xvfb-run-eepp b/projects/scripts/xvfb-run-eepp
index 446fe14fb..7ef9d9324 100755
--- a/projects/scripts/xvfb-run-eepp
+++ b/projects/scripts/xvfb-run-eepp
@@ -103,6 +103,26 @@ claim_display() {
         return 0
     fi
 
+    # Older versions of this script claimed displays from a command substitution,
+    # which left lock directories behind because LOCKDIR was set in a subshell.
+    # Recover those stale claims, but never touch a lock owned by a live process.
+    if [ -f "$lock/pid" ]; then
+        oldpid=$(cat "$lock/pid" 2>/dev/null || true)
+        case "$oldpid" in
+            *[!0-9]*|'') oldpid= ;;
+        esac
+        if [ -z "$oldpid" ] || ! kill -0 "$oldpid" >/dev/null 2>&1; then
+            rm -f "$lock/pid" 2>/dev/null || true
+            rmdir "$lock" 2>/dev/null || true
+            if mkdir "$lock" 2>/dev/null; then
+                LOCKDIR=$lock
+                SERVERNUM=$n
+                echo "$$" > "$LOCKDIR/pid" 2>/dev/null || true
+                return 0
+            fi
+        fi
+    fi
+
     return 1
 }
 
@@ -110,7 +130,6 @@ find_and_claim_display() {
     i=$SERVERNUM
     while [ "$i" -le "$MAX_SERVERNUM" ]; do
         if ! is_display_busy "$i" && claim_display "$i"; then
-            echo "$i"
             return 0
         fi
         i=$((i + 1))
@@ -195,8 +214,9 @@ if ! command -v Xvfb >/dev/null 2>&1; then
 fi
 
 if [ "$AUTO_SERVERNUM" = yes ]; then
-    if ! SERVERNUM=$(find_and_claim_display); then
-        error "could not find a free X display between $SERVERNUM and $MAX_SERVERNUM"
+    START_SERVERNUM=$SERVERNUM
+    if ! find_and_claim_display; then
+        error "could not find a free X display between $START_SERVERNUM and $MAX_SERVERNUM"
         exit 4
     fi
 else