morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #5189] fix(hopper): preserve an explicit display number zero in the X11 helper #3181

New issue

Closed

opened 2026-10-31 00:37:35 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:37:35 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 19, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5189

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Other: packaged Hopper X11 helper

Observed behavior

The packaged scripts/hopper-demo-x11.py helper accepts --display-number 0 but replaces it with a random display. Its collision check can then reject an unrelated random display without checking the requested :0. This affects the helper's explicit display option; the normal Node probe and launch wrappers omit this option.

All four assertions passed on each tested baseline. With the synthetic random value fixed, explicit zero authorized :123, explicit one authorized :1 without calling randomness, and omission authorized :123. The fake authorization command returned failure intentionally, so those runs returned 71 with xauth_failed; these are fixture stop signals, not observed real authorization failures. When only X123 was simulated as occupied, explicit zero returned 70 with address_collision before authorization and never checked X0.

The relevant output for the two zero cases was:

{"authorized_displays": [":123"], "checked": ["/tmp/.X11-unix/X123", "/tmp/.X123-lock"], "exit_code": 71, "random_calls": 1, "reason": "xauth_failed", "requested": "0", "simulated_random_collision": false}
{"authorized_displays": [], "checked": ["/tmp/.X11-unix/X123"], "exit_code": 70, "random_calls": 1, "reason": "address_collision", "requested": "0", "simulated_random_collision": true}

This is an executed isolated reproduction of the helper's selection and error handling with faked external commands and filesystem observations. Real Hopper, Xvfb and xauth were not run; no real display binding or completed provider launch is claimed.

Reproduction

At any tested source baseline listed below, save the following as check-display.py in the repository root and run PYTHONDONTWRITEBYTECODE=1 python3 check-display.py. It executes the unmodified helper parser, main, probe, display selection and authorization-command construction with synthetic external boundaries. It prevents process creation, simulates display availability, fixes the random value, and intentionally makes the fake authorization command fail after recording its display argument. No X11 installation or Hopper binary is needed. Remove check-display.py after the check.

import contextlib
import io
import json
from pathlib import Path
import sys
import tempfile
import types
from unittest.mock import patch

helper = types.ModuleType("hopper_display_check")
source = Path("scripts/hopper-demo-x11.py")
exec(compile(source.read_bytes(), str(source), "exec"), helper.__dict__)

for requested, occupied in [("0", False), ("1", False), (None, False), ("0", True)]:
    calls = []
    checked = []
    def exists(path):
        text = str(path)
        if text.startswith("/tmp/.X"):
            checked.append(text)
            return occupied and text == "/tmp/.X11-unix/X123"
        raise AssertionError("unexpected filesystem check: " + text)
    def fake_run(command, **kwargs):
        calls.append(command[4])
        return types.SimpleNamespace(returncode=1)
    argv = [str(source), "--probe", "--xauth", "fake-xauth"]
    if requested is not None:
        argv += ["--display-number", requested]
    with tempfile.TemporaryDirectory() as isolated:
        real_tempdir = tempfile.TemporaryDirectory
        def owned_tempdir(*args, **kwargs):
            kwargs["dir"] = isolated
            return real_tempdir(*args, **kwargs)
        output = io.StringIO()
        with patch.object(sys, "argv", argv), \
             patch.object(helper.secrets, "randbelow", return_value=23) as random, \
             patch.object(helper.Path, "exists", exists), \
             patch.object(helper.subprocess, "run", fake_run), \
             patch.object(helper.subprocess, "Popen", side_effect=AssertionError("unexpected process")), \
             patch.object(helper.tempfile, "TemporaryDirectory", owned_tempdir), \
             patch.object(helper, "socket_directory_state", return_value=("1777", False, True)), \
             patch.object(helper, "is_wsl", return_value=False), \
             contextlib.redirect_stderr(output):
            exit_code = helper.main()
        diagnostic = json.loads(output.getvalue().split("=",1)[1])
    result = dict(requested=requested, simulated_random_collision=occupied,
                  authorized_displays=calls, checked=checked, random_calls=random.call_count,
                  exit_code=exit_code, reason=diagnostic["reason"])
    print(json.dumps(result, sort_keys=True))
    if occupied:
        assert result["authorized_displays"] == [] and exit_code == 70
        assert result["checked"] == ["/tmp/.X11-unix/X123"]
    else:
        assert calls == [":1" if requested == "1" else ":123"]
        assert exit_code == 71 and diagnostic["reason"] == "xauth_failed"
        assert random.call_count == (0 if requested == "1" else 1)

Expected behavior

An explicit display number zero should select :0 for collision checking and authorization. An explicit nonzero number should select that number; only an omitted option should select a random display. A collision at a different display should not reject the explicitly selected zero.

Impact and scope

The failure is bounded to direct use of the packaged helper with an explicit zero display number. The normal Node probe and launch wrappers omit this option. No core workflow blockage, data loss, or real-provider launch failure was established.

REA version

Tested source revisions: hunt baseline 9aca543f46085f502dbc5445002e129e5d2974e9; current upstream main 2e5171e391b83f8a467d0861fd452c1f78621fd8; latest published release rea-agents-6.7.0 at 27dc42d10249f0410904a75c0997842eb03b931c. The helper is byte-identical across these baselines.

Runtime and platform

Checks used Python 3.9.6 on macOS arm64, with no installed Node/npm dependency or real-provider execution. The supported Linux helper's selection path was executed under the explicitly described synthetic boundaries; Linux server behavior was not tested.

Target and artifact context

The packaged Python helper CLI, with synthetic display occupancy and a fake authorization command. No binary or device target was used.

Interface

Other: packaged Python helper CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Expected source: The repository's input-preservation rule requires preserving caller-selected inputs. The helper's argument parser accepts the integer option without excluding zero, and --help advertises it. Its main dispatch sends --probe into run_probe and start_xvfb without a zero-exclusion guard.
Failure source: The observations above come from the inline fixture executing the production path. In display selection and collision handling, the truthiness default substitutes randomness for zero, while the explicit-selection collision branch still treats zero as supplied.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The primary violation evidence is the executed selection/authorization-argument and collision behavior with simulated external boundaries. Four isolated cases passed per baseline, including two zero failure observations and the nonzero/omitted controls. Source references establish applicability and reachability. No real-provider reproduction was run.

Restoration check

With the same fixture inputs and synthetic random value, explicit zero should check X0 and authorize :0 without consulting randomness. Simulating occupation of only X123 should leave the explicit-zero selection unaffected. Explicit one should continue selecting :1 without randomness, and omission should continue selecting the synthetic random display. The current fixture encodes the observed broken behavior, so its zero-case expectations must change when judging restoration. Real-provider startup remains a separate validation scope.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A valid explicit input is silently replaced, and the helper can return an address-collision failure for a different display. This is bounded to the helper CLI's explicit zero option; the normal wrappers omit it, and no core workflow blockage or data loss was established.
Workaround status: unverified
Workaround basis: No workaround preserving an exact :0 selection was verified. Selecting a different nonzero display changes the requested result and is not counted as restoration of that selection.

Originally created by @coygeek on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5189 ## Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ## Area Other: packaged Hopper X11 helper ## Observed behavior The packaged `scripts/hopper-demo-x11.py` helper accepts `--display-number 0` but replaces it with a random display. Its collision check can then reject an unrelated random display without checking the requested `:0`. This affects the helper's explicit display option; the normal Node probe and launch wrappers omit this option. All four assertions passed on each tested baseline. With the synthetic random value fixed, explicit zero authorized `:123`, explicit one authorized `:1` without calling randomness, and omission authorized `:123`. The fake authorization command returned failure intentionally, so those runs returned 71 with `xauth_failed`; these are fixture stop signals, not observed real authorization failures. When only `X123` was simulated as occupied, explicit zero returned 70 with `address_collision` before authorization and never checked `X0`. The relevant output for the two zero cases was: ```json {"authorized_displays": [":123"], "checked": ["/tmp/.X11-unix/X123", "/tmp/.X123-lock"], "exit_code": 71, "random_calls": 1, "reason": "xauth_failed", "requested": "0", "simulated_random_collision": false} {"authorized_displays": [], "checked": ["/tmp/.X11-unix/X123"], "exit_code": 70, "random_calls": 1, "reason": "address_collision", "requested": "0", "simulated_random_collision": true} ``` This is an executed isolated reproduction of the helper's selection and error handling with faked external commands and filesystem observations. Real Hopper, Xvfb and xauth were not run; no real display binding or completed provider launch is claimed. ## Reproduction At any tested source baseline listed below, save the following as `check-display.py` in the repository root and run `PYTHONDONTWRITEBYTECODE=1 python3 check-display.py`. It executes the unmodified helper parser, `main`, probe, display selection and authorization-command construction with synthetic external boundaries. It prevents process creation, simulates display availability, fixes the random value, and intentionally makes the fake authorization command fail after recording its display argument. No X11 installation or Hopper binary is needed. Remove `check-display.py` after the check. ```python import contextlib import io import json from pathlib import Path import sys import tempfile import types from unittest.mock import patch helper = types.ModuleType("hopper_display_check") source = Path("scripts/hopper-demo-x11.py") exec(compile(source.read_bytes(), str(source), "exec"), helper.__dict__) for requested, occupied in [("0", False), ("1", False), (None, False), ("0", True)]: calls = [] checked = [] def exists(path): text = str(path) if text.startswith("/tmp/.X"): checked.append(text) return occupied and text == "/tmp/.X11-unix/X123" raise AssertionError("unexpected filesystem check: " + text) def fake_run(command, **kwargs): calls.append(command[4]) return types.SimpleNamespace(returncode=1) argv = [str(source), "--probe", "--xauth", "fake-xauth"] if requested is not None: argv += ["--display-number", requested] with tempfile.TemporaryDirectory() as isolated: real_tempdir = tempfile.TemporaryDirectory def owned_tempdir(*args, **kwargs): kwargs["dir"] = isolated return real_tempdir(*args, **kwargs) output = io.StringIO() with patch.object(sys, "argv", argv), \ patch.object(helper.secrets, "randbelow", return_value=23) as random, \ patch.object(helper.Path, "exists", exists), \ patch.object(helper.subprocess, "run", fake_run), \ patch.object(helper.subprocess, "Popen", side_effect=AssertionError("unexpected process")), \ patch.object(helper.tempfile, "TemporaryDirectory", owned_tempdir), \ patch.object(helper, "socket_directory_state", return_value=("1777", False, True)), \ patch.object(helper, "is_wsl", return_value=False), \ contextlib.redirect_stderr(output): exit_code = helper.main() diagnostic = json.loads(output.getvalue().split("=",1)[1]) result = dict(requested=requested, simulated_random_collision=occupied, authorized_displays=calls, checked=checked, random_calls=random.call_count, exit_code=exit_code, reason=diagnostic["reason"]) print(json.dumps(result, sort_keys=True)) if occupied: assert result["authorized_displays"] == [] and exit_code == 70 assert result["checked"] == ["/tmp/.X11-unix/X123"] else: assert calls == [":1" if requested == "1" else ":123"] assert exit_code == 71 and diagnostic["reason"] == "xauth_failed" assert random.call_count == (0 if requested == "1" else 1) ``` ## Expected behavior An explicit display number zero should select `:0` for collision checking and authorization. An explicit nonzero number should select that number; only an omitted option should select a random display. A collision at a different display should not reject the explicitly selected zero. ## Impact and scope The failure is bounded to direct use of the packaged helper with an explicit zero display number. The normal Node probe and launch wrappers omit this option. No core workflow blockage, data loss, or real-provider launch failure was established. ## REA version Tested source revisions: hunt baseline `9aca543f46085f502dbc5445002e129e5d2974e9`; current upstream main `2e5171e391b83f8a467d0861fd452c1f78621fd8`; latest published release `rea-agents-6.7.0` at `27dc42d10249f0410904a75c0997842eb03b931c`. The helper is byte-identical across these baselines. ## Runtime and platform Checks used Python 3.9.6 on macOS arm64, with no installed Node/npm dependency or real-provider execution. The supported Linux helper's selection path was executed under the explicitly described synthetic boundaries; Linux server behavior was not tested. ## Target and artifact context The packaged Python helper CLI, with synthetic display occupancy and a fake authorization command. No binary or device target was used. ## Interface Other: packaged Python helper CLI ## Earliest failure stage Setup or provider admission ## Relevant output or evidence - Expected source: The repository's [input-preservation rule](https://github.com/morluto/rea/blob/2e5171e391b83f8a467d0861fd452c1f78621fd8/AGENTS.md#L13) requires preserving caller-selected inputs. The helper's [argument parser](https://github.com/morluto/rea/blob/2e5171e391b83f8a467d0861fd452c1f78621fd8/scripts/hopper-demo-x11.py#L113-L131) accepts the integer option without excluding zero, and `--help` advertises it. Its [main dispatch](https://github.com/morluto/rea/blob/2e5171e391b83f8a467d0861fd452c1f78621fd8/scripts/hopper-demo-x11.py#L816-L836) sends `--probe` into `run_probe` and `start_xvfb` without a zero-exclusion guard. - Failure source: The observations above come from the inline fixture executing the production path. In [display selection and collision handling](https://github.com/morluto/rea/blob/2e5171e391b83f8a467d0861fd452c1f78621fd8/scripts/hopper-demo-x11.py#L576-L645), the truthiness default substitutes randomness for zero, while the explicit-selection collision branch still treats zero as supplied. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The primary violation evidence is the executed selection/authorization-argument and collision behavior with simulated external boundaries. Four isolated cases passed per baseline, including two zero failure observations and the nonzero/omitted controls. Source references establish applicability and reachability. No real-provider reproduction was run. ## Restoration check With the same fixture inputs and synthetic random value, explicit zero should check `X0` and authorize `:0` without consulting randomness. Simulating occupation of only `X123` should leave the explicit-zero selection unaffected. Explicit one should continue selecting `:1` without randomness, and omission should continue selecting the synthetic random display. The current fixture encodes the observed broken behavior, so its zero-case expectations must change when judging restoration. Real-provider startup remains a separate validation scope. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A valid explicit input is silently replaced, and the helper can return an address-collision failure for a different display. This is bounded to the helper CLI's explicit zero option; the normal wrappers omit it, and no core workflow blockage or data loss was established. - Workaround status: unverified - Workaround basis: No workaround preserving an exact `:0` selection was verified. Selecting a different nonzero display changes the requested result and is not counted as restoration of that selection.

bsakaguchi closed this issue

2026-10-31 00:37:36 +00:00

bsakaguchi commented

2026-10-31 00:37:36 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 19, 2026):

Classic falsy-zero trap — using the truthiness of the display number as "option omitted" conflates an explicit :0 with the default. The fix should use a sentinel/None default so explicit zero goes through the same selection, collision check and authorization path as any other explicit value, and only omission picks a random display. The fixture already encodes the broken behavior, so updating its zero-case expectations plus adding a case where X123 is occupied but explicit zero is requested would lock the regression in.

@XinwangTao commented on GitHub (Oct 19, 2026): Classic falsy-zero trap — using the truthiness of the display number as "option omitted" conflates an explicit :0 with the default. The fix should use a sentinel/None default so explicit zero goes through the same selection, collision check and authorization path as any other explicit value, and only omission picks a random display. The fixture already encodes the broken behavior, so updating its zero-case expectations plus adding a case where X123 is occupied but explicit zero is requested would lock the regression in.

bsakaguchi referenced this issue

2026-10-31 00:42:16 +00:00

[GH-ISSUE #5923] [Bug] Trace comparisons report no first divergence for ordering failures and missing journals #3600

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#3181

No description provided.

Rows
Columns