Skip to content

find correct path for .exe on custom wine/proton#802

Open
Smuger wants to merge 11 commits intoutkarshdalal:masterfrom
Smuger:custom-wine-find-exe-path
Open

find correct path for .exe on custom wine/proton#802
Smuger wants to merge 11 commits intoutkarshdalal:masterfrom
Smuger:custom-wine-find-exe-path

Conversation

@Smuger
Copy link

@Smuger Smuger commented Mar 10, 2026


Summary by cubic

Adds an in-app Input Debug HUD to trace Android→X11→Wine input in real time, and improves custom Wine/Proton support by ensuring winhandler.exe/wfm.exe are present. Also fixes a keyboard mapping reply size issue and publishes a debug test APK to a GitHub pre-release on each CI run.

  • New Features

    • Added InputDebugOverlay/InputDebugLogger with live event log, counters, pointer coords, mouse mode (REL/ABS), Wine debug status/layout; parses Wine logs and shows X11/Wine key/mouse activity; toggle via Quick Menu (“Input Debug”).
    • Hooked Android key/mouse and X11 keyboard events into the logger; added Keyboard.lookupXKeycode and WinHandler.isRunning() to support accurate mapping and status display.
  • Bug Fixes

    • For custom Wine/Proton prefixes, ensure winhandler.exe and wfm.exe exist in .wine/drive_c/windows; if missing, extract only these from container_pattern_gamenative.tzst.
    • Correct GetKeyboardMapping reply length to avoid out-of-bounds.

Written for commit bf75434. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Adds a fallback extraction to ensure required Windows executables are present in custom Wine/Proton prefixes, improving game compatibility.
  • Chores

    • Simplifies the build/release flow by removing multi-step signing, adopting a streamlined APK build/verify flow, using dynamic artifact handling, auto-generating changelog entries for manual builds, creating timestamped tags and GitHub Releases with the APK attached, and removing the prior notification step.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a conditional fallback extraction in XServerScreen that, after general container-pattern extraction, checks for winhandler.exe and wfm.exe in the container Windows prefix and extracts them from container_pattern_gamenative.tzst if missing; also rewrites the release CI to remove keystore signing and create a GitHub Release with the built APK.

Changes

Cohort / File(s) Summary
Executable Fallback Extraction
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
After existing pattern extraction, verify presence of C:\windows\winhandler.exe and C:\windows\wfm.exe in the container prefix; if absent, open container_pattern_gamenative.tzst and extract only those files into the Windows prefix.
Release CI — Workflow rewrite
.github/workflows/app-release-signed.yml
Removes keystore configuration and APK signing steps; simplifies build to produce/verify APK, generates a timestamped tag, creates a GitHub prerelease and attaches the APK as an asset, and adjusts changelog handling (including manual dispatch case).

Sequence Diagram(s)

sequenceDiagram
  participant App as XServerScreen
  participant FS as Container .wine/drive_c/windows
  participant Archive as container_pattern_gamenative.tzst
  participant Extractor as Extraction Routine

  App->>FS: Check for winhandler.exe and wfm.exe
  alt files present
    FS-->>App: Files exist (no extraction)
  else missing
    App->>Archive: Open archive and filter for winhandler.exe,wfm.exe
    Archive->>Extractor: Stream matched files
    Extractor->>FS: Write files to .wine/drive_c/windows
    FS-->>App: Files written
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through tar and tzst light,
Found two exes hiding out of sight.
Dropped them in Windows, snug and neat,
Tagged a release, made the build complete,
🥕🎮

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'find correct path for .exe on custom wine/proton' clearly summarizes the main change: detecting and extracting missing GameNative executables for custom Wine/Proton builds.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

@Smuger
Copy link
Author

Smuger commented Mar 10, 2026

I'm currently hitting a problem while running custom proton build where I have winhandler.exe and wfm.exe in xuser-STEAM_65540

/imagefs/home/xuser-STEAM_65540/.wine/drive_c/windows/winhandler.exe

and not in

/imagefs/home/xuser/.wine/drive_c/windows/

03-10 21:26:22.672 10152 11076 I System.out: 0034:trace:process:find_exe_file looking for L"C:\\windows\\system32\\winedevice.exe" in L"C:\\windows\\system32;.;C:\\windows\\system32;C:\\windows\\system;C:\\windows;C:\\Program Files (x86)\\Steam;C:\\windows\\system32;C:\\windows;C:\\windows\\system32\\wbem;C:\\windows\\system32\\WindowsPowershell\\v1.0"
03-10 21:26:22.688 10152 11076 I System.out: 0034:trace:module:LdrGetDllHandleEx flags 0, load_path 00000000006F6670, dll_characteristics 0000000000000000, name 000000000011DFD8, base 000000000011DD68.
03-10 21:26:22.689 10152 11076 I System.out: 0034:trace:module:LdrGetDllHandleEx L"C:\\windows\\system32\\winedevice.exe" -> 0000000000000000 (load path L"C:\\windows\\system32;C:\\windows\\system32;C:\\windows\\system;C:\\windows;C:\\Program Files (x86)\\Steam;.;C:\\windows\\system32;C:\\windows;C:\\windows\\system32\\wbem;C:\\windows\\system32\\WindowsPowershell\\v1.0")
03-10 21:26:22.689 10152 11076 I System.out: 0034:trace:module:get_load_order looking for L"C:\\windows\\system32\\winedevice.exe"
03-10 21:26:22.689 10152 11076 I System.out: 0034:trace:module:get_load_order got hardcoded default for L"winedevice.exe"

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)

3166-3179: Check extraction result for easier debugging of custom Wine/Proton fallback path.

The TarCompressorUtils.extract() method returns false when extraction fails (e.g., due to IOException), but the result is ignored here. If extraction fails for any reason, this fallback path will silently fail, making debugging harder for users with custom Wine/Proton builds.

Consider checking the result and logging on failure:

Suggested improvement
     if (gameNativeExes.any { !File(windowsDir, it).exists() }) {
         Timber.i("GameNative executables missing from container, extracting from container_pattern_gamenative.tzst")
-        TarCompressorUtils.extract(
+        val extracted = TarCompressorUtils.extract(
             TarCompressorUtils.Type.ZSTD,
             context.assets,
             "container_pattern_gamenative.tzst",
             container.rootDir,
             object : OnExtractFileListener {
                 override fun onExtractFile(file: File, size: Long): File? {
                     return if (file.name in gameNativeExes) file else null
                 }
             },
         )
+        if (!extracted) {
+            Timber.w("Failed to extract GameNative executables")
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around
lines 3166 - 3179, The extraction result from TarCompressorUtils.extract is
ignored; update the block that calls TarCompressorUtils.extract (the call using
context.assets, "container_pattern_gamenative.tzst", container.rootDir and the
OnExtractFileListener that filters by gameNativeExes) to capture the
boolean/return value and handle failures: if extract returns false (or throws),
log a clear error with Timber.e including the filename
"container_pattern_gamenative.tzst", the target container.rootDir, and any
caught exception details, and surface the failure (e.g., return/notify/throw) so
the missing gameNativeExes case is not silently ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 3166-3179: The extraction result from TarCompressorUtils.extract
is ignored; update the block that calls TarCompressorUtils.extract (the call
using context.assets, "container_pattern_gamenative.tzst", container.rootDir and
the OnExtractFileListener that filters by gameNativeExes) to capture the
boolean/return value and handle failures: if extract returns false (or throws),
log a clear error with Timber.e including the filename
"container_pattern_gamenative.tzst", the target container.rootDir, and any
caught exception details, and surface the failure (e.g., return/notify/throw) so
the missing gameNativeExes case is not silently ignored.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 713ba477-dfd6-49c2-9372-d884435d53a0

📥 Commits

Reviewing files that changed from the base of the PR and between c3c7e1e and 2dca60c.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/app-release-signed.yml:
- Around line 57-59: The CI step writes prod-keystore.properties missing the
required storeFile entry, breaking the four-field keystore contract used by
app/build.gradle.kts (storeFile, storePassword, keyAlias, keyPassword); update
the workflow to also write a storeFile line (pointing to the keystore path
referenced by the Gradle config, e.g. app/keystores/prod.keystore or the
appropriate environment variable) into app/keystores/prod-keystore.properties so
all four properties are present when loaded.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a7cfb2f5-c4be-4b44-b65a-065d412b3459

📥 Commits

Reviewing files that changed from the base of the PR and between 2dca60c and 3723408.

📒 Files selected for processing (1)
  • .github/workflows/app-release-signed.yml

Comment on lines +57 to +59
echo "keyAlias=$PROD_KEYSTORE_KEY_ALIAS" > app/keystores/prod-keystore.properties
echo "keyPassword=$PROD_KEYSTORE_KEY_PASSWORD" >> app/keystores/prod-keystore.properties
echo "storePassword=$PROD_KEYSTORE_STORE_PASSWORD" >> app/keystores/prod-keystore.properties
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add storeFile to prod-keystore.properties.

app/build.gradle.kts:16-46 uses a four-field keystore properties contract: storeFile, storePassword, keyAlias, and keyPassword. This step now writes only the last three, so anything loading prod-keystore.properties with the same contract will fail to resolve app/keystores/prod.keystore.

Proposed fix
-        echo "keyAlias=$PROD_KEYSTORE_KEY_ALIAS" > app/keystores/prod-keystore.properties
-        echo "keyPassword=$PROD_KEYSTORE_KEY_PASSWORD" >> app/keystores/prod-keystore.properties
-        echo "storePassword=$PROD_KEYSTORE_STORE_PASSWORD" >> app/keystores/prod-keystore.properties
+        {
+          echo "storeFile=keystores/prod.keystore"
+          echo "keyAlias=$PROD_KEYSTORE_KEY_ALIAS"
+          echo "keyPassword=$PROD_KEYSTORE_KEY_PASSWORD"
+          echo "storePassword=$PROD_KEYSTORE_STORE_PASSWORD"
+        } > app/keystores/prod-keystore.properties
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
echo "keyAlias=$PROD_KEYSTORE_KEY_ALIAS" > app/keystores/prod-keystore.properties
echo "keyPassword=$PROD_KEYSTORE_KEY_PASSWORD" >> app/keystores/prod-keystore.properties
echo "storePassword=$PROD_KEYSTORE_STORE_PASSWORD" >> app/keystores/prod-keystore.properties
{
echo "storeFile=keystores/prod.keystore"
echo "keyAlias=$PROD_KEYSTORE_KEY_ALIAS"
echo "keyPassword=$PROD_KEYSTORE_KEY_PASSWORD"
echo "storePassword=$PROD_KEYSTORE_STORE_PASSWORD"
} > app/keystores/prod-keystore.properties
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 57 - 59, The CI step
writes prod-keystore.properties missing the required storeFile entry, breaking
the four-field keystore contract used by app/build.gradle.kts (storeFile,
storePassword, keyAlias, keyPassword); update the workflow to also write a
storeFile line (pointing to the keystore path referenced by the Gradle config,
e.g. app/keystores/prod.keystore or the appropriate environment variable) into
app/keystores/prod-keystore.properties so all four properties are present when
loaded.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/app-release-signed.yml">

<violation number="1" location=".github/workflows/app-release-signed.yml:72">
P1: Release creation step requires write token permissions, but workflow does not declare `permissions: contents: write`, causing failures under read-only default `GITHUB_TOKEN` settings.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/app-release-signed.yml (1)

71-80: Declare contents: write explicitly.

This step depends on GITHUB_TOKEN being able to create tags and releases. Without an explicit permissions block, it only works while the repository default token policy stays permissive.

Suggested permissions block
 jobs:
   build:
     runs-on: ubuntu-latest
+    permissions:
+      contents: write
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 71 - 80, The "Create
Release" step using softprops/action-gh-release@v2 requires the GITHUB_TOKEN to
have write permission to create tags/releases; add an explicit permissions block
(e.g., permissions: contents: write) to the workflow or the job that contains
the "Create Release" step so the token can create releases reliably; ensure the
permissions block is placed at workflow or job scope above the step and not just
rely on repository defaults, leaving the "Create Release" step (uses:
softprops/action-gh-release@v2) and its GITHUB_TOKEN env intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/app-release-signed.yml:
- Around line 71-79: Update the release workflow to build the pluvia-signed
artifact instead of the debug-signed one: replace the Gradle task currently
invoking :app:bundleRelease with :app:bundleReleaseSigned so the produced
universal.apk is signed with the pluvia signing config (the existing
release-signed build variant), and ensure the workflow step that
uploads/publishes universal.apk uses the artifact produced by
:app:bundleReleaseSigned.
- Around line 67-80: The "Create tag name", "Create Release", and "Post to
Discord" workflow steps must be gated to the master branch to prevent
tags/releases from non-master refs; add an if: github.ref == 'refs/heads/master'
condition to each of those steps (the steps titled "Create tag name", "Create
Release", and "Post to Discord") so they only run when github.ref equals
'refs/heads/master', while leaving artifact upload steps unchanged.

---

Nitpick comments:
In @.github/workflows/app-release-signed.yml:
- Around line 71-80: The "Create Release" step using
softprops/action-gh-release@v2 requires the GITHUB_TOKEN to have write
permission to create tags/releases; add an explicit permissions block (e.g.,
permissions: contents: write) to the workflow or the job that contains the
"Create Release" step so the token can create releases reliably; ensure the
permissions block is placed at workflow or job scope above the step and not just
rely on repository defaults, leaving the "Create Release" step (uses:
softprops/action-gh-release@v2) and its GITHUB_TOKEN env intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c80e81aa-1464-4454-b15f-58db34b0dc75

📥 Commits

Reviewing files that changed from the base of the PR and between 1b946b4 and 7dfdf48.

📒 Files selected for processing (1)
  • .github/workflows/app-release-signed.yml

Comment on lines +67 to +80
- name: Create tag name
id: tag
run: echo "name=build-$(date -u +%Y%m%d-%H%M%S)-${GITHUB_SHA::7}" >> "$GITHUB_OUTPUT"

- name: Create Release
uses: softprops/action-gh-release@v2
with:
tag_name: ${{ steps.tag.outputs.name }}
name: "Build ${{ steps.tag.outputs.name }}"
prerelease: true
body: ${{ steps.changelog.outputs.log }}
files: universal.apk
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat .github/workflows/app-release-signed.yml

Repository: utkarshdalal/GameNative

Length of output: 2850


Gate public tag/release creation to master branch.

workflow_dispatch on line 12 allows manual runs from any ref via the UI/API. This enables creating tags and public releases from unmerged feature branches. Add if: github.ref == 'refs/heads/master' to the "Create tag name" step (line 68), "Create Release" step (line 72), and "Post to Discord" step (line 82) to restrict these operations to master. Artifact uploads will remain available for ad-hoc branch builds.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 67 - 80, The "Create
tag name", "Create Release", and "Post to Discord" workflow steps must be gated
to the master branch to prevent tags/releases from non-master refs; add an if:
github.ref == 'refs/heads/master' condition to each of those steps (the steps
titled "Create tag name", "Create Release", and "Post to Discord") so they only
run when github.ref equals 'refs/heads/master', while leaving artifact upload
steps unchanged.

Comment on lines +71 to +79
- name: Create Release
uses: softprops/action-gh-release@v2
with:
tag_name: ${{ steps.tag.outputs.name }}
name: "Build ${{ steps.tag.outputs.name }}"
prerelease: true
body: ${{ steps.changelog.outputs.log }}
files: universal.apk
env:
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the app-release-signed.yml workflow file
cat -n .github/workflows/app-release-signed.yml | head -100

Repository: utkarshdalal/GameNative

Length of output: 3466


🏁 Script executed:

# Now let's look at the build.gradle.kts file to check signing configuration
cat -n app/build.gradle.kts | head -150

Repository: utkarshdalal/GameNative

Length of output: 6207


🏁 Script executed:

# Let's search for where universal.apk is generated and what build variant it uses
rg "universal.apk" -n

Repository: utkarshdalal/GameNative

Length of output: 49


🏁 Script executed:

# Search for signingConfigs definitions in gradle files
rg "signingConfigs" -n -A 3

Repository: utkarshdalal/GameNative

Length of output: 1369


Switch to building :app:bundleReleaseSigned instead of :app:bundleRelease to use the pluvia signing key.

Line 41 builds :app:bundleRelease, which uses the debug signing config (app/build.gradle.kts line 118). Publishing this debug-signed universal.apk to the public release channel breaks the upgrade path for users currently on the pluvia-signed variant. A separate release-signed build variant exists (lines 120–124) that correctly uses the pluvia signing config—update the workflow to use :app:bundleReleaseSigned instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 71 - 79, Update the
release workflow to build the pluvia-signed artifact instead of the debug-signed
one: replace the Gradle task currently invoking :app:bundleRelease with
:app:bundleReleaseSigned so the produced universal.apk is signed with the pluvia
signing config (the existing release-signed build variant), and ensure the
workflow step that uploads/publishes universal.apk uses the artifact produced by
:app:bundleReleaseSigned.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
.github/workflows/app-release-signed.yml (2)

40-41: ⚠️ Potential issue | 🔴 Critical

Debug-signed APK breaks upgrade path for existing users.

This builds the release build type, which uses signingConfig = signingConfigs.getByName("debug") per app/build.gradle.kts:118. Users who installed the app with the pluvia-signed variant cannot upgrade to a debug-signed APK due to signature mismatch.

If the intent is to avoid production signing for prereleases (per PR description), consider either:

  1. Using assembleRelease-signed with proper keystore configuration to maintain upgrade compatibility
  2. Clearly documenting that these prerelease builds require a fresh install and cannot upgrade existing installations

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 40 - 41, The workflow
currently runs ./gradlew :app:assembleRelease which builds the Release variant
signed with signingConfigs.getByName("debug") (see app/build.gradle.kts) and
breaks upgrade path; update the workflow to produce a release-signed APK (e.g.,
run the assemble task that uses your production keystore such as
:app:assembleRelease-signed or adjust the Gradle variant/task to use the proper
signingConfig) or explicitly document that artifacts are debug-signed and
require a fresh install; locate references to assembleRelease in the GitHub
Actions job and the signingConfig usage in app/build.gradle.kts
(signingConfigs.getByName("debug")) and change the task or signing configuration
accordingly so prereleases remain upgrade-compatible.

70-83: ⚠️ Potential issue | 🟠 Major

Gate tag/release creation to master branch.

workflow_dispatch (line 12) allows manual runs from any branch. Without branch guards, tags and releases can be created from unmerged feature branches. Add conditions to restrict these operations:

Proposed fix
     - name: Create tag name
       id: tag
+      if: github.ref == 'refs/heads/master'
       run: echo "name=build-$(date -u +%Y%m%d-%H%M%S)-${GITHUB_SHA::7}" >> "$GITHUB_OUTPUT"

     - name: Create Release
+      if: github.ref == 'refs/heads/master'
       uses: softprops/action-gh-release@v2
       with:
         tag_name: ${{ steps.tag.outputs.name }}

Artifact uploads (line 51) will remain available for ad-hoc branch builds without creating public releases.

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 70 - 83, The
tag/release creation steps currently run for manual workflow_dispatch runs from
any branch; gate the tag creation (step id "tag") and the "Create Release" step
(softprops/action-gh-release@v2) so they only run for the master branch by
adding an if condition such as if: "github.ref == 'refs/heads/master'" (or move
the condition to the parent job) so tag_name and the release are only created
when github.ref is the master branch; keep artifact upload steps unchanged so
ad-hoc branch builds still produce artifacts.
🧹 Nitpick comments (1)
.github/workflows/app-release-signed.yml (1)

43-49: Step name is misleading.

This step verifies the APK has any valid signature, but since assembleRelease uses the debug signing config, it validates a debug signature rather than a production signature. Consider renaming to "Verify APK build" or adding a comment clarifying this validates debug-signed output.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/app-release-signed.yml around lines 43 - 49, Rename or
clarify the GitHub Actions step currently titled "Verify APK is signed" to avoid
implying production signing; either change the step name to something like
"Verify APK build" or add a brief inline comment that this verifies whatever
signature is present (e.g., the debug signature produced by assembleRelease) so
reviewers understand this does not confirm production signing, and update any
related references (the step title and surrounding comments) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/app-release-signed.yml:
- Around line 40-41: The workflow currently runs ./gradlew :app:assembleRelease
which builds the Release variant signed with signingConfigs.getByName("debug")
(see app/build.gradle.kts) and breaks upgrade path; update the workflow to
produce a release-signed APK (e.g., run the assemble task that uses your
production keystore such as :app:assembleRelease-signed or adjust the Gradle
variant/task to use the proper signingConfig) or explicitly document that
artifacts are debug-signed and require a fresh install; locate references to
assembleRelease in the GitHub Actions job and the signingConfig usage in
app/build.gradle.kts (signingConfigs.getByName("debug")) and change the task or
signing configuration accordingly so prereleases remain upgrade-compatible.
- Around line 70-83: The tag/release creation steps currently run for manual
workflow_dispatch runs from any branch; gate the tag creation (step id "tag")
and the "Create Release" step (softprops/action-gh-release@v2) so they only run
for the master branch by adding an if condition such as if: "github.ref ==
'refs/heads/master'" (or move the condition to the parent job) so tag_name and
the release are only created when github.ref is the master branch; keep artifact
upload steps unchanged so ad-hoc branch builds still produce artifacts.

---

Nitpick comments:
In @.github/workflows/app-release-signed.yml:
- Around line 43-49: Rename or clarify the GitHub Actions step currently titled
"Verify APK is signed" to avoid implying production signing; either change the
step name to something like "Verify APK build" or add a brief inline comment
that this verifies whatever signature is present (e.g., the debug signature
produced by assembleRelease) so reviewers understand this does not confirm
production signing, and update any related references (the step title and
surrounding comments) accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0b2247e-b7ca-475e-9a4a-cb493ed995c3

📥 Commits

Reviewing files that changed from the base of the PR and between 7dfdf48 and 3b60520.

📒 Files selected for processing (1)
  • .github/workflows/app-release-signed.yml

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/src/main/java/com/winlator/xserver/requests/KeyboardRequests.java">

<violation number="1" location="app/src/main/java/com/winlator/xserver/requests/KeyboardRequests.java:30">
P2: `firstKeycode` is treated as signed in index calculation, causing incorrect (zero) keyboard mappings for valid keycodes >= 128.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/src/main/java/com/winlator/xserver/requests/KeyboardRequests.java">

<violation number="1">
P1: Direct array indexing in `getKeyboardMapping` removed prior bounds checks, allowing malformed keycode/count values to crash with `ArrayIndexOutOfBoundsException`.</violation>

<violation number="2">
P1: GetKeyboardMapping response length and indexing were changed to per-keycode count instead of per-keysym count, causing truncated/misaligned keyboard mappings.</violation>
</file>

<file name="app/src/main/java/com/winlator/winhandler/WinHandler.java">

<violation number="1" location="app/src/main/java/com/winlator/winhandler/WinHandler.java:91">
P2: isRunning() can return a stale ready state because initReceived is never cleared on stop/start; after a restart it may report ready before a new INIT handshake.</violation>
</file>

<file name="app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt:113">
P2: User-visible strings are hardcoded across the new overlay instead of using Android string resources, reducing localization and maintainability.</violation>

<violation number="2" location="app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt:267">
P2: LazyColumn key uses millisecond timestamp, which is not guaranteed unique under burst input and can cause item identity/state collisions.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

this.inputControlsView = view;
}

public boolean isRunning() {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 12, 2026

Choose a reason for hiding this comment

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

P2: isRunning() can return a stale ready state because initReceived is never cleared on stop/start; after a restart it may report ready before a new INIT handshake.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/winhandler/WinHandler.java, line 91:

<comment>isRunning() can return a stale ready state because initReceived is never cleared on stop/start; after a restart it may report ready before a new INIT handshake.</comment>

<file context>
@@ -84,11 +84,14 @@ public class WinHandler {
         this.inputControlsView = view;
     }
 
+    public boolean isRunning() {
+        return running && initReceived;
+    }
</file context>
Fix with Cubic

@@ -0,0 +1,462 @@
package app.gamenative.ui.component
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 12, 2026

Choose a reason for hiding this comment

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

P2: User-visible strings are hardcoded across the new overlay instead of using Android string resources, reducing localization and maintainability.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt, line 113:

<comment>User-visible strings are hardcoded across the new overlay instead of using Android string resources, reducing localization and maintainability.</comment>

<file context>
@@ -0,0 +1,462 @@
+                        )
+                        Spacer(Modifier.width(8.dp))
+                        Text(
+                            text = "Input Debug",
+                            style = MaterialTheme.typography.titleLarge.copy(
+                                fontWeight = FontWeight.Bold,
</file context>
Fix with Cubic

.weight(1f),
verticalArrangement = Arrangement.spacedBy(2.dp),
) {
items(events, key = { it.timestamp }) { event ->
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 12, 2026

Choose a reason for hiding this comment

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

P2: LazyColumn key uses millisecond timestamp, which is not guaranteed unique under burst input and can cause item identity/state collisions.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt, line 267:

<comment>LazyColumn key uses millisecond timestamp, which is not guaranteed unique under burst input and can cause item identity/state collisions.</comment>

<file context>
@@ -0,0 +1,462 @@
+                            .weight(1f),
+                        verticalArrangement = Arrangement.spacedBy(2.dp),
+                    ) {
+                        items(events, key = { it.timestamp }) { event ->
+                            EventRow(event)
+                        }
</file context>
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:584">
P2: Keyboard debug listener is attached but only detached on overlay dismiss, so non-dismiss teardown paths can leak/stack listeners.</violation>
</file>

<file name="app/src/main/java/app/gamenative/ui/component/InputDebugLogger.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/component/InputDebugLogger.kt:327">
P2: Mouse log parsing was narrowed to `call_event_handler`-gated patterns, causing valid Wine mouse/cursor traces to be missed in the debug overlay.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

}

// Mouse events from Wine X11 event handler
line.contains("call_event_handler") && line.contains("MotionNotify") -> {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 13, 2026

Choose a reason for hiding this comment

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

P2: Mouse log parsing was narrowed to call_event_handler-gated patterns, causing valid Wine mouse/cursor traces to be missed in the debug overlay.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/InputDebugLogger.kt, line 327:

<comment>Mouse log parsing was narrowed to `call_event_handler`-gated patterns, causing valid Wine mouse/cursor traces to be missed in the debug overlay.</comment>

<file context>
@@ -211,46 +323,51 @@ object InputDebugLogger {
-            // Mouse/cursor traces
-            line.contains("cursor:") || (line.contains("mouse:") && line.contains("trace:")) -> {
+            // Mouse events from Wine X11 event handler
+            line.contains("call_event_handler") && line.contains("MotionNotify") -> {
                 wineEventCount.value++
                 val short = extractAfterFunction(line)
</file context>
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt:182">
P2: Refactor removed key debug diagnostics: overlay no longer shows actual WINEDEBUG channels or input-path visibility, reducing actionable troubleshooting data.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

)
InfoChip(
"WineDbg",
if (wineDebugActive) "ON" else "OFF",
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 13, 2026

Choose a reason for hiding this comment

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

P2: Refactor removed key debug diagnostics: overlay no longer shows actual WINEDEBUG channels or input-path visibility, reducing actionable troubleshooting data.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/component/InputDebugOverlay.kt, line 182:

<comment>Refactor removed key debug diagnostics: overlay no longer shows actual WINEDEBUG channels or input-path visibility, reducing actionable troubleshooting data.</comment>

<file context>
@@ -77,243 +81,195 @@ fun InputDebugOverlay(
-                        value = if (wineDebugActive) wineDebugChannels.take(40) else "DISABLED",
-                        color = if (wineDebugActive) PluviaTheme.colors.accentSuccess else PluviaTheme.colors.accentDanger,
+                        "WineDbg",
+                        if (wineDebugActive) "ON" else "OFF",
+                        if (wineDebugActive) PluviaTheme.colors.accentSuccess else PluviaTheme.colors.accentDanger,
                     )
</file context>
Fix with Cubic

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant