feat: add WSL wrapper runner and performance docs#10
Open
julianhelpertz wants to merge 2 commits intoaidanqm:mainfrom
Open
feat: add WSL wrapper runner and performance docs#10julianhelpertz wants to merge 2 commits intoaidanqm:mainfrom
julianhelpertz wants to merge 2 commits intoaidanqm:mainfrom
Conversation
Reviewer's GuideAdds a new WSL/Linux Bash runner that extracts and patches the Codex macOS DMG, prepares native modules against the correct Electron version, and launches Codex with tunable display/performance options, alongside README updates documenting WSL usage, required DMG provisioning, and WSLg refresh-rate tuning. Sequence diagram for WSL user running Codex via scripts_run_shsequenceDiagram
actor WSLUser
participant Bash as scripts_run_sh
participant OS as WSL_Linux_OS
participant SevenZ as ensure_7z
participant APT as apt_get_dpkg_deb
participant Node as Node_npm_npx
participant Electron as Electron_runtime
participant CodexCLI as Codex_CLI
WSLUser->>Bash: ./scripts/run.sh --dmg ./Codex.dmg --reuse
Bash->>OS: need_cmd node, npm, npx
OS-->>Bash: binaries available
Bash->>Bash: resolve_dmg_path
Bash->>SevenZ: ensure_7z
alt 7z in PATH or system
SevenZ-->>Bash: path to 7z
else 7z not installed
SevenZ->>APT: apt-get download 7zip
APT-->>SevenZ: 7zip deb package
SevenZ->>APT: dpkg-deb -x to local tools dir
SevenZ-->>Bash: local 7z path under work/tools
end
opt REUSE == 0
Bash->>SevenZ: extract app.asar and app.asar.unpacked
Bash->>Node: npx @electron/asar extract app.asar -> APP_DIR
Bash->>Bash: copy_dir_contents app.asar.unpacked -> APP_DIR
end
Bash->>Node: patch_preload via node script
Node-->>Bash: preload.js patched (process bridge)
Bash->>Node: read package.json
Node-->>Bash: ELECTRON_VERSION, native module versions, metadata
Bash->>Node: prepare native modules in NATIVE_DIR
Node-->>Bash: better-sqlite3.node, pty.node built for Electron
Bash->>Bash: copy native modules into APP_DIR
alt NO_LAUNCH == 1
Bash-->>WSLUser: exit after preparation only
else launch Electron
Bash->>CodexCLI: resolve_codex_cli_path (find codex)
CodexCLI-->>Bash: path to codex
Bash->>Electron: exec electron binary
activate Electron
Electron->>Electron: read APP_DIR
Electron->>CodexCLI: spawn Codex CLI as needed
CodexCLI-->>Electron: responses for app features
Electron-->>WSLUser: Codex UI displayed via WSLg/Wayland/X11
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- When reading dependency versions from package.json, consider explicitly checking that BETTER_VERSION and PTY_VERSION are non-empty before using them in npm install, and fail fast with a clear error message if they’re missing to avoid confusing install errors.
- The 7z bootstrap logic currently assumes an apt-based distro and a package named
7zip; you might want to allow overriding the 7z binary via an environment variable (e.g., CODEX_7Z_PATH) or make the package name configurable to better support non-Debian WSL/Linux setups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When reading dependency versions from package.json, consider explicitly checking that BETTER_VERSION and PTY_VERSION are non-empty before using them in npm install, and fail fast with a clear error message if they’re missing to avoid confusing install errors.
- The 7z bootstrap logic currently assumes an apt-based distro and a package named `7zip`; you might want to allow overriding the 7z binary via an environment variable (e.g., CODEX_7Z_PATH) or make the package name configurable to better support non-Debian WSL/Linux setups.
## Individual Comments
### Comment 1
<location path="scripts/run.sh" line_range="215" />
<code_context>
+BS_DST="$APP_DIR/node_modules/better-sqlite3/build/Release/better_sqlite3.node"
+PTY_DST_PRE="$APP_DIR/node_modules/node-pty/prebuilds/$NODE_PTY_ARCH"
+PTY_DST_REL="$APP_DIR/node_modules/node-pty/build/Release"
+if [[ "$NO_LAUNCH" -eq 1 && "$REUSE" -eq 1 && -f "$BS_DST" && ( -f "$PTY_DST_PRE/pty.node" || -f "$PTY_DST_REL/pty.node" ) ]]; then
+ echo "Native modules already present in app. Skipping rebuild."
+else
</code_context>
<issue_to_address>
**issue (bug_risk):** The reuse check for native modules is gated on NO_LAUNCH, which seems inverted and causes extra rebuilds when launching.
With the current condition, native modules are only skipped when NO_LAUNCH=1 and REUSE=1. When actually launching (NO_LAUNCH=0) with REUSE=1 and valid artifacts, we still rebuild, negating reuse on repeated launches. Recommend removing the NO_LAUNCH check and relying on REUSE plus the existing file checks for better-sqlite3 and node-pty.
</issue_to_address>
### Comment 2
<location path="scripts/run.sh" line_range="46-47" />
<code_context>
+ (
+ cd "$pkg_dir"
+ rm -f 7zip_*.deb
+ apt-get download 7zip >/dev/null
+ deb="$(ls -1t 7zip_*.deb | head -n 1)"
+ dpkg-deb -x "$deb" "$out_dir"
+ )
+ fi
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 7zip bootstrap flow can fail in a non-obvious way if apt-get download fails or produces no .deb.
If `apt-get download 7zip` fails (network, missing package, repo misconfig, etc.), `ls -1t 7zip_*.deb` will fail and `deb` will be empty, causing a confusing `dpkg-deb` error later. Please add an explicit check that the download succeeded and that `deb` is non-empty, and fail early with a clear message about the package download problem before calling `dpkg-deb`.
```suggestion
cd "$pkg_dir"
rm -f 7zip_*.deb
if ! apt-get download 7zip >/dev/null 2>&1; then
die "Failed to download 7zip package via apt-get. Check network connectivity and APT configuration, then retry."
fi
deb="$(ls -1t 7zip_*.deb 2>/dev/null | head -n 1)"
if [[ -z "$deb" ]]; then
die "apt-get download 7zip did not produce a 7zip_*.deb package in $pkg_dir"
fi
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| BS_DST="$APP_DIR/node_modules/better-sqlite3/build/Release/better_sqlite3.node" | ||
| PTY_DST_PRE="$APP_DIR/node_modules/node-pty/prebuilds/$NODE_PTY_ARCH" | ||
| PTY_DST_REL="$APP_DIR/node_modules/node-pty/build/Release" | ||
| if [[ "$NO_LAUNCH" -eq 1 && "$REUSE" -eq 1 && -f "$BS_DST" && ( -f "$PTY_DST_PRE/pty.node" || -f "$PTY_DST_REL/pty.node" ) ]]; then |
There was a problem hiding this comment.
issue (bug_risk): The reuse check for native modules is gated on NO_LAUNCH, which seems inverted and causes extra rebuilds when launching.
With the current condition, native modules are only skipped when NO_LAUNCH=1 and REUSE=1. When actually launching (NO_LAUNCH=0) with REUSE=1 and valid artifacts, we still rebuild, negating reuse on repeated launches. Recommend removing the NO_LAUNCH check and relying on REUSE plus the existing file checks for better-sqlite3 and node-pty.
Comment on lines
+46
to
+47
| cd "$pkg_dir" | ||
| rm -f 7zip_*.deb |
There was a problem hiding this comment.
suggestion (bug_risk): 7zip bootstrap flow can fail in a non-obvious way if apt-get download fails or produces no .deb.
If apt-get download 7zip fails (network, missing package, repo misconfig, etc.), ls -1t 7zip_*.deb will fail and deb will be empty, causing a confusing dpkg-deb error later. Please add an explicit check that the download succeeded and that deb is non-empty, and fail early with a clear message about the package download problem before calling dpkg-deb.
Suggested change
| cd "$pkg_dir" | |
| rm -f 7zip_*.deb | |
| cd "$pkg_dir" | |
| rm -f 7zip_*.deb | |
| if ! apt-get download 7zip >/dev/null 2>&1; then | |
| die "Failed to download 7zip package via apt-get. Check network connectivity and APT configuration, then retry." | |
| fi | |
| deb="$(ls -1t 7zip_*.deb 2>/dev/null | head -n 1)" | |
| if [[ -z "$deb" ]]; then | |
| die "apt-get download 7zip did not produce a 7zip_*.deb package in $pkg_dir" | |
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/run.shto run Codex from a macOSCodex.dmgCodex.dmgthemselves.wslgconfig) and recommended Wayland launch command for better UXChanges
scripts/run.sh:app.asar+app.asar.unpacked) on WSL/Linuxbetter-sqlite3andnode-ptyCODEX_OZONE_PLATFORM, cursor, window position/size, etc.)README.md:Codex.dmgTest Plan
bash -n scripts/run.sh./scripts/run.sh --dmg ./Codex.dmg --no-launchCODEX_OZONE_PLATFORM=wayland ./scripts/run.sh --dmg ./Codex.dmg --reuseCloses #7
Summary by Sourcery
Add a WSL/Linux runner script for launching the Codex macOS DMG and expand documentation for using the wrapper on Windows and WSL with performance-related guidance.
New Features:
Documentation: