[PR #5214] [MERGED] fix(hopper): preserve an explicit display number zero in the X11 helper #6819

Closed
opened 2026-11-01 21:43:15 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5214
Author: @musaraza004
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/hopper-explicit-display-zero


📝 Commits (1)

  • 0a5bc95 fix(hopper): preserve an explicit display number zero in the X11 helper

📊 Changes

3 files changed (+114 additions, -1 deletions)

View changed files

📝 scripts/hopper-demo-x11.py (+5 -1)
📝 tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts (+40 -0)
➕ tests/fixtures/hopperDisplaySelection.py (+69 -0)

📄 Description

Summary

Fixes #5189.

The packaged scripts/hopper-demo-x11.py helper accepted --display-number 0 but swapped it for a random display, because start_xvfb read the option's truthiness. Now an explicit zero goes through the same selection, collision check and authorization as any other explicit display.

Problem and expected behavior

  • Trigger: --display-number 0.
  • Before: args.display_number or (100 + secrets.randbelow(900)) treated 0 as omitted. The helper checked and authorized a random display such as :123. If that random display was occupied, it raised address_collision without ever checking X0. Because the later guards use args.display_number is not None, that random collision was then treated as an explicit-display failure.
  • Expected: the helper picks a random display only when the option is omitted. An explicit 0 checks /tmp/.X11-unix/X0 and /tmp/.X0-lock, then authorizes :0.

Change and scope

The random display is now chosen only when args.display_number is None. This matches the is not None checks the function already uses for collisions. The normal Node probe and launch wrappers don't pass this option, so their behavior is unchanged. Contribution scope decision: not applicable (an ordinary fix within existing boundaries).

Contract and boundary impact

  • Semantic owner and earliest changed stage: start_xvfb display selection in scripts/hopper-demo-x11.py.
  • Contribution scope fit and linked maintainer decision: ordinary fix, not applicable.
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: Linux Hopper demo helper only. Explicit --display-number 0 now targets :0.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details: the explicit display is honored. Collision handling for explicit displays is unchanged (fail without retry).
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process. (This helper launches Xvfb. The change only affects which display it picks.)
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: new fixture tests/fixtures/hopperDisplaySelection.py, plus a Linux-only case in tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts that follows the existing Python-fixture pattern. The fixture loads the real helper and fakes Path.exists, secrets.randbelow (fixed at 23, giving :123) and authorize_display. The fake authorize_display raises xauth_failed so the run stops before Xvfb. The fixture covers five cases: explicit zero, explicit zero with only X123 occupied, explicit zero with X0 occupied, explicit one, and omitted.

  • Base reproduction (executed against the unfixed helper): the fixture reproduced the issue exactly.

    • Explicit zero: authorized [":123"], random_calls 1.
    • Explicit zero with X123 occupied: address_collision, only X123 checked.
    • Explicit zero with X0 occupied: still authorized :123.

    The new test fails on the base revision.

  • After the fix:

    • Explicit zero authorizes :0 and checks X0 and .X0-lock, with 0 random calls.
    • Explicit zero with X0 occupied returns address_collision.
    • Explicit one and omitted are unchanged.
  • Remaining proof gaps: real Hopper, Xvfb and xauth were not run. I ran the fixture directly but didn't run vitest locally.

Validation performed

  • python tests/fixtures/hopperDisplaySelection.py scripts/hopper-demo-x11.py (Windows, Python 3.14), before the fix: reproduced the issue's output. After the fix: output matches the test's expected JSON.
  • python3 tests/fixtures/hopperDisplaySelection.py scripts/hopper-demo-x11.py (WSL Linux) after the fix: exit 0, output matches the test's expected JSON.
  • Not run: vitest (dependencies not installed locally). Relying on CI for the boundary test.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. An explicit --display-number 0 now means :0, which is what it was documented to mean.
  • Real Hopper/Ghidra, browser, or OS coverage: none. The fixture uses simulated paths and commands.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none. The collision check still runs before authorization.

Author readiness

Human owner: @musaraza004

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5214 **Author:** [@musaraza004](https://github.com/musaraza004) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/hopper-explicit-display-zero` --- ### 📝 Commits (1) - [`0a5bc95`](https://github.com/morluto/rea/commit/0a5bc95ffe5d36ed8f1c2ff094479317c970ec5e) fix(hopper): preserve an explicit display number zero in the X11 helper ### 📊 Changes **3 files changed** (+114 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `scripts/hopper-demo-x11.py` (+5 -1) 📝 `tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts` (+40 -0) ➕ `tests/fixtures/hopperDisplaySelection.py` (+69 -0) </details> ### 📄 Description ## Summary Fixes #5189. The packaged `scripts/hopper-demo-x11.py` helper accepted `--display-number 0` but swapped it for a random display, because `start_xvfb` read the option's truthiness. Now an explicit zero goes through the same selection, collision check and authorization as any other explicit display. ## Problem and expected behavior - Trigger: `--display-number 0`. - Before: `args.display_number or (100 + secrets.randbelow(900))` treated `0` as omitted. The helper checked and authorized a random display such as `:123`. If that random display was occupied, it raised `address_collision` without ever checking `X0`. Because the later guards use `args.display_number is not None`, that random collision was then treated as an explicit-display failure. - Expected: the helper picks a random display only when the option is omitted. An explicit `0` checks `/tmp/.X11-unix/X0` and `/tmp/.X0-lock`, then authorizes `:0`. ## Change and scope The random display is now chosen only when `args.display_number is None`. This matches the `is not None` checks the function already uses for collisions. The normal Node probe and launch wrappers don't pass this option, so their behavior is unchanged. Contribution scope decision: not applicable (an ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `start_xvfb` display selection in `scripts/hopper-demo-x11.py`. - Contribution scope fit and linked maintainer decision: ordinary fix, not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: Linux Hopper demo helper only. Explicit `--display-number 0` now targets `:0`. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: the explicit display is honored. Collision handling for explicit displays is unchanged (fail without retry). - Generated metadata, package, or installation impact: none. Expected effects: - [ ] Read existing files or captured data. - [x] Launch or attach to a process. (This helper launches Xvfb. The change only affects which display it picks.) - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: new fixture `tests/fixtures/hopperDisplaySelection.py`, plus a Linux-only case in `tests/boundary/providers/hopper/linuxPrivateDisplay.test.ts` that follows the existing Python-fixture pattern. The fixture loads the real helper and fakes `Path.exists`, `secrets.randbelow` (fixed at 23, giving `:123`) and `authorize_display`. The fake `authorize_display` raises `xauth_failed` so the run stops before Xvfb. The fixture covers five cases: explicit zero, explicit zero with only `X123` occupied, explicit zero with `X0` occupied, explicit one, and omitted. - Base reproduction (executed against the unfixed helper): the fixture reproduced the issue exactly. - Explicit zero: `authorized [":123"]`, `random_calls 1`. - Explicit zero with `X123` occupied: `address_collision`, only `X123` checked. - Explicit zero with `X0` occupied: still authorized `:123`. The new test fails on the base revision. - After the fix: - Explicit zero authorizes `:0` and checks `X0` and `.X0-lock`, with 0 random calls. - Explicit zero with `X0` occupied returns `address_collision`. - Explicit one and omitted are unchanged. - Remaining proof gaps: real Hopper, Xvfb and xauth were not run. I ran the fixture directly but didn't run vitest locally. ## Validation performed - `python tests/fixtures/hopperDisplaySelection.py scripts/hopper-demo-x11.py` (Windows, Python 3.14), before the fix: reproduced the issue's output. After the fix: output matches the test's expected JSON. - `python3 tests/fixtures/hopperDisplaySelection.py scripts/hopper-demo-x11.py` (WSL Linux) after the fix: exit 0, output matches the test's expected JSON. - Not run: `vitest` (dependencies not installed locally). Relying on CI for the boundary test. ## Compatibility, safety, and release - Breaking changes or migration steps: none. An explicit `--display-number 0` now means `:0`, which is what it was documented to mean. - Real Hopper/Ghidra, browser, or OS coverage: none. The fixture uses simulated paths and commands. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. The collision check still runs before authorization. ## Author readiness Human owner: @musaraza004 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:43:15 +00:00
Sign in to join this conversation.
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#6819
No description provided.