-
Notifications
You must be signed in to change notification settings - Fork 32
🤖 fix: AppImage CLI argument handling #1161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
c3a4fff to
eb41316
Compare
Fix CLI subcommands (server, api, run) not working when invoked from packaged AppImage. The issue was incorrect argv offset detection. Problem: - In packaged Electron apps, argv = [app, ...args] (first arg at index 1) - In dev mode (electron .), argv = [electron, ".", ...args] (index 2) - In bun/node, argv = [node, script, ...args] (index 2) The original code used process.argv[2] unconditionally, which broke packaged apps. The initial fix used process.defaultApp which is undefined in both packaged Electron AND bun/node, breaking non-Electron CLI. Solution: - Use isPackagedElectron = isElectron && !process.defaultApp - Only packaged Electron uses index 1; all others use index 2 - Update Commander parse mode in server.ts and run.ts Additional improvements: - Hide 'run' command from packaged app help (not bundled) - Friendly error when trying to run 'mux run' from AppImage - Remove '(requires Electron)' suffix in Electron context --- _Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking: `high`_
eb41316 to
40ea583
Compare
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
40ea583 to
f22885d
Compare
- Extract isCommandAvailable() to argv.ts for testable command availability - Fix run command check: use isElectron (not isPackagedElectron) - Add 5 unit tests for isCommandAvailable() - bunx electron . run now shows friendly error instead of crash
f22885d to
daf28bc
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
) ## Summary Fix `./mux.AppImage --no-sandbox` and other Electron flags not working - they were incorrectly falling through to CLI help instead of launching the desktop app. ## Problem After the previous CLI argv fix (#1161), Electron flags like `--no-sandbox` in packaged AppImage were treated as unknown CLI arguments: ``` ./mux.AppImage --no-sandbox # showed CLI help instead of launching desktop ``` ## Root Cause `isElectronLaunchArg()` immediately returned `false` for all packaged Electron flags, not distinguishing between: - **CLI flags** (`--help`, `--version`) → should show CLI help - **Electron flags** (`--no-sandbox`, `--disable-gpu`) → should launch desktop ## Solution - Add `CLI_GLOBAL_FLAGS` constant listing flags that should show CLI help - Update `isElectronLaunchArg()` to return `true` for non-CLI flags in packaged mode - Add runtime sanity check (dev mode only) to warn if flags get out of sync - Add tests for the new behavior ## Manual Test Results | Command | Result | |---------|--------| | `./mux.AppImage --no-sandbox` | ✅ Launches desktop | | `./mux.AppImage --disable-gpu` | ✅ Launches desktop | | `./mux.AppImage --help` | ✅ Shows CLI help | | `./mux.AppImage --version` | ✅ Shows version | | `./mux.AppImage server --help` | ✅ Shows server options | | `bunx electron . --no-sandbox` | ✅ Launches desktop (dev mode) | --- _Generated with `mux` • Model: `anthropic:claude-sonnet-4-20250514` • Thinking: `low`_
Summary
Fix CLI subcommands (
server,api,run) not working when invoked from packaged AppImage.Problem
When running
./mux.AppImage serveror./mux.AppImage --help, the commands weren't recognized and the app would either launch the desktop GUI or fail.Root cause: Incorrect argv offset detection across different execution contexts:
isElectronprocess.defaultAppbun muxelectron ../mux.AppImageThe original code used
process.argv[2]unconditionally, which broke packaged apps.Solution
Created
src/cli/argv.tswith testable pure functions:detectCliEnvironment()- Returns{ isElectron, isPackagedElectron, firstArgIndex }getParseOptions()- Returns Commander parse options ({ from: "electron" | "node" })getSubcommand()- Gets subcommand at correct argv indexgetArgsAfterSplice()- Gets remaining args for subcommand handlersisCommandAvailable()- Checks if a command is available in current environmentKey insight: Use
isPackagedElectron = isElectron && !process.defaultAppto correctly identify packaged apps.Manual Test Results
Bun/Node CLI
bun src/cli/index.ts --helpbun src/cli/index.ts server --helpbun src/cli/index.ts run --helpbun dist/cli/index.js api --helpElectron Dev Mode
bunx electron . server --helpbunx electron . api --helpbunx electron . runbunx electron .Packaged AppImage
./release/mux-*-x86_64.AppImage --help./release/mux-*-x86_64.AppImage server --help./release/mux-*-x86_64.AppImage api --help./release/mux-*-x86_64.AppImage run./release/mux-*-x86_64.AppImageGenerated with
mux• Model:anthropic:claude-sonnet-4-20250514• Thinking:low