Skip to content

Conversation

@Coldaine
Copy link
Owner

@Coldaine Coldaine commented Dec 25, 2025

User description

Summary

Removes hardcoded DISPLAY=:99 from text_injection_tests job. The self-hosted runner has a live KDE Plasma session - display should be inherited from environment.

Follow-up to PR #319.

🤖 Generated with Claude Code


PR Type

Bug fix


Description

  • Remove hardcoded DISPLAY=:99 from CI environment variables

  • Remove xvfb-action step that's incompatible with Fedora/Nobara

  • Use live KDE Plasma session display from self-hosted runner

  • Simplify CI workflow by removing unnecessary virtual display setup


Diagram Walkthrough

flowchart LR
  A["CI Workflow"] -->|Remove xvfb-action| B["Eliminate virtual display"]
  A -->|Remove DISPLAY=:99| C["Use live KDE session"]
  B --> D["Simplify self-hosted runner"]
  C --> D
Loading

File Walkthrough

Relevant files
Configuration changes
ci.yml
Remove xvfb and hardcoded display configuration                   

.github/workflows/ci.yml

  • Removed hardcoded DISPLAY: :99 environment variable and replaced with
    comment explaining display inheritance
  • Removed entire Start Xvfb step that used GabrielBB/xvfb-action@v1
  • Removed dependency installation commands for xclip, xdotool,
    at-spi2-core, and liberation-sans-fonts
  • Kept D-Bus session setup which remains necessary for the self-hosted
    runner
+1/-10   

Coldaine and others added 2 commits December 24, 2025 21:09
The self-hosted runner has a live KDE Plasma session with real display.
Xvfb is unnecessary and the action also uses apt-get internally which
doesn't exist on Fedora/Nobara.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copilot AI review requested due to automatic review settings December 25, 2025 05:11
@Coldaine Coldaine enabled auto-merge (squash) December 25, 2025 05:11
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Coldaine Coldaine merged commit 9929763 into main Dec 25, 2025
9 of 11 checks passed
@Coldaine Coldaine deleted the fix/remove-xvfb branch December 25, 2025 05:11
@qodo-code-review
Copy link

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing DISPLAY guard: The workflow now relies on an inherited DISPLAY without verifying it is set, which may
cause GUI-dependent tests to fail without clear, early diagnostics on runners lacking a
live session.

Referred Code
# DISPLAY inherited from runner's live KDE session (do NOT hardcode :99)
RUST_LOG: debug
RUST_TEST_TIME_UNIT: 10000
RUST_TEST_TIME_INTEGRATION: 30000
SCCACHE_DIR: $HOME/.cache/sccache

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Re-add installation of removed dependencies

Add a new step to install GUI test dependencies like xclip and xdotool. These
installations were removed along with the Xvfb step, and explicitly installing
them makes the workflow more reliable and self-contained.

.github/workflows/ci.yml [273-277]

 - name: Setup ColdVox
   uses: ./.github/actions/setup-coldvox
+
+- name: Install GUI test dependencies
+  run: |
+    set -euo pipefail
+    echo "Installing dependencies for hardware integration tests..."
+    sudo dnf install -y xclip xdotool at-spi2-core liberation-sans-fonts
 
 - name: Setup D-Bus Session
   run: |

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that removing the Xvfb step also removed necessary dependency installations. Making these dependencies explicit in the workflow is a crucial best practice for reliability and maintainability, even on self-hosted runners.

Medium
  • More

@qodo-code-review
Copy link

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Re-add installation of critical dependencies

Re-add the installation step for GUI test dependencies (xclip, xdotool, etc.)
that was removed along with the Xvfb step. This ensures the workflow explicitly
declares its dependencies for better reproducibility.

.github/workflows/ci.yml [273-277]

 - name: Setup ColdVox
   uses: ./.github/actions/setup-coldvox
+
+- name: Install GUI test dependencies
+  run: |
+    set -euo pipefail
+    echo "Installing dependencies for hardware integration tests..."
+    sudo dnf install -y xclip xdotool at-spi2-core liberation-sans-fonts
 
 - name: Setup D-Bus Session
   run: |

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that removing the dependency installation step makes the CI job less reproducible and prone to failure if the runner environment changes. Re-adding it significantly improves the workflow's robustness.

Medium
Check that DISPLAY is defined

Add a CI step to verify that the DISPLAY environment variable is set. The step
should cause the job to fail early if the variable is missing, preventing
obscure errors later in the GUI tests.

.github/workflows/ci.yml [235]

 # DISPLAY inherited from runner's live KDE session (do NOT hardcode :99)
+- name: Ensure DISPLAY is set
+  run: |
+    if [ -z "${DISPLAY:-}" ]; then
+      echo "ERROR: DISPLAY is not set. Aborting."
+      exit 1
+    fi

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: This suggestion improves the CI workflow's robustness by adding a check to ensure the DISPLAY variable is set, which is a new assumption. This provides a clear failure message if the runner is misconfigured, aiding debuggability.

Low
  • More

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the CI configuration for the text_injection_tests job to use the self-hosted runner's native display environment instead of a virtual display server. The change removes the hardcoded DISPLAY=:99 environment variable and the Xvfb virtual display setup, allowing the tests to run on the runner's existing KDE Plasma session.

Key Changes:

  • Removed hardcoded DISPLAY=:99 environment variable, allowing display to be inherited from the runner's environment
  • Eliminated the "Start Xvfb" step that was setting up a virtual X display server
  • Added clear documentation comment explaining the display inheritance strategy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants