Conversation
…esh_cached_status` with test coverage and improved CLI handling
…-progress and app quitting scenarios
…dling with tests
…nstallation retry methods
…ling and clarified installation behaviors
…s to prevent conflicts with new installs
# Conflicts: # CHANGELOG.md # scripts/patch-linux-window-ui.js
Tests in `codex_cli` and `app` mutate process-wide env vars (`HOME`,
`PATH`, `NVM_DIR`, `CODEX_CLI_PATH`, display sockets) so they can drive
`command_path_env`, `npm_program`, and `hydrate_session_bus_env`
deterministically. Cargo runs tests in parallel, so without a shared
lock those mutations race across threads — on a developer machine with
nvm installed, `which npm` resolves to the real toolchain before the
test fixture instead of the temp-dir fake, and the assertion fails:
assertion `left == right` failed
left: "/home/user/.nvm/versions/node/v22.22.0/bin/npm"
right: "/tmp/.tmp.../bin/npm"
The CI runner did not see this because `rust-and-smoke` doesn't run
`setup-node`, so there's no nvm to leak in. Any local contributor with
nvm could not run `cargo test` though.
Add a shared `test_util::env_lock()` helper that hands out a process-
global mutex guard, and acquire it from each test that touches the
env. Also apply `cargo clippy --fix` to drop four
`uninlined_format_args` warnings in `codex_cli.rs` that fire on rustc
1.80+ — CI's bundled clippy still treats that lint as allow-by-default
so they passed silently, but newer toolchains would flip the workflow
red.
…h feature, renderer, and install flow patches
…r-use-atspi-ergonomics feat(computer-use): improve Linux AT-SPI action ergonomics
fix(nix): update Codex DMG hash
fix(nix): expose runtime libraries
Merge upstream/main through 410e5fc while preserving this fork's codex-app identity, XDG/FHS package layout, updater naming, and package-version contract. Local validation: git diff --cached --check; cargo fmt --check --manifest-path updater/Cargo.toml; bash -n scripts/lib/asar-patch.sh tests/scripts_smoke.sh launcher/start.sh.template; node --test scripts/patch-linux-window-ui.test.js; bash tests/scripts_smoke.sh; make check; make test; make build-app; make pacman. Co-authored-by: Codex <noreply@openai.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a persisted ChangesCodex CLI Status & Reconciliation
Webview Server Lifecycle & Stale-Detection
Linux Quit Guard & Electron Patch
Click Target Resolution & AT‑SPI Actions
Nix Packaging, Deterministic Asar & Installer Changes
Sequence Diagram(s)sequenceDiagram
participant Launcher as Launcher
participant Updater as Updater Daemon
participant CLI as Codex CLI
participant Desktop as Desktop Notifier
Launcher->>Launcher: Detect & stop stale webview server
Launcher->>Launcher: Ensure webview server running
Updater->>CLI: reconcile_if_present()
alt runtime CLI resolved
CLI->>CLI: preflight (may install/upgrade)
CLI-->>Updater: updated=true/false
else runtime CLI not present
CLI->>CLI: mark CliStatus::NotInstalled
CLI-->>Updater: updated=false
end
Updater->>Updater: check state.cli_status
alt NotInstalled
Updater->>Desktop: emit "cli_missing" notification
else Installed
Updater->>Desktop: emit regular status
end
sequenceDiagram
participant App as Electron App
participant Patch as Quit Guard Handler
participant Tray as Tray UI
participant Window as Window UI
App->>Patch: before-quit event
Patch->>Patch: set codexLinuxQuitInProgress = true
Tray->>Patch: check codexLinuxIsQuitInProgress()
alt quit in progress
Tray->>Tray: skip hide-to-tray and menu refresh
else normal
Tray->>Tray: perform hide-to-tray/menu ops
end
Window->>Patch: second-instance / launch-action
alt quit in progress
Window->>Window: short-circuit launch-action handling
else normal
Window->>Window: process launch args
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be56a49595
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6099399017
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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 `@launcher/start.sh.template`:
- Around line 831-836: Replace the raw internal error message from notify_error
that exposes the function name and exit code with a user-actionable message:
when stop_stale_webview_server returns non-zero (check stop_status), call
notify_error with a friendly explanation such as "Unable to stop the webview
helper process; please close any running webview windows or reboot the machine
and check logs for details" (optionally mention where logs/config are), then
exit with stop_status as before; update the notify_error invocation that
currently references stop_stale_webview_server and stop_status to use this new
user-facing text while preserving the exit "$stop_status" behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fff4b7d2-1bf8-4f24-8fef-6982b5259d99
📒 Files selected for processing (4)
computer-use-linux/src/server.rsflake.nixlauncher/start.sh.templatescripts/patch-linux-window-ui.test.js
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
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 `@launcher/start.sh.template`:
- Around line 833-835: The notify_error call uses an undefined variable
CODEX_LINUX_LOG_FILE causing an empty path in the message; update the message to
reference the actual launcher log variable LOG_FILE (use "$LOG_FILE" with
quotes) so when stop_status != 0 the notify_error in the stop block and any
related messages use LOG_FILE instead of CODEX_LINUX_LOG_FILE; check the
stop_status conditional and the notify_error invocation to ensure consistent
quoting and variable name usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab34f44b-567f-47fb-874d-3f2174a61bf5
📒 Files selected for processing (1)
launcher/start.sh.template
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Stale CodeRabbit request addressed by 7bb38a6; final CodeRabbit status and all required checks are passing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
nisavid has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ! grep -q "NixOS Electron library path" "${installDir}/start.sh"; then | ||
| ${pkgs.gnused}/bin/sed -i '2i\ | ||
| # NixOS Electron library path for dlopen()ed GL/EGL libraries.\ | ||
| export LD_LIBRARY_PATH="${electronLibPath}:${runtimeLibPath}:''${LD_LIBRARY_PATH:-}"' "${installDir}/start.sh" | ||
| fi | ||
| if ! grep -q "codex_nixos_add_runtime_library_dirs" "${installDir}/start.sh"; then |
| running_app_is_active && return 0 | ||
|
|
||
| while pid="$(stale_webview_server_pid)"; do | ||
| cwd="$(readlink -f "/proc/$pid/cwd" 2>/dev/null || true)" |
## Summary Follow-up upstream sync after PR #17 because upstream advanced from `410e5fc` to `55e11db`. - Merge upstream `main` through `55e11db5924cf9b2c17a5c3fe63f071db6cab0a1` while preserving upstream commit identity. - Adapt upstream Linux app update flow under this fork's `codex-app` and `codex-app-updater` contracts. - Update the fork divergence baseline to `55e11db5924cf9b2c17a5c3fe63f071db6cab0a1`. - Address review feedback so `--inspect` stays read-only and avoids full build deps, desktop actions keep the packaged updater path, update install retries work after app exit, relaunch uses the configured app launcher, rebuild installs recover on late failures, patch reports survive malformed or missing JSON, process detection ignores other users/helpers, interrupted installs surface failures, and install-ready missing artifacts return failures. ## Sync Ledger - Fork target verified: `nisavid/codex-app-linux`. - Direct upstream: `upstream/main` at `55e11db5924cf9b2c17a5c3fe63f071db6cab0a1`. - Previous synced baseline: `410e5fc` from PR #17. - Policy read: `AGENTS.md`, user-global `syncing-forks-with-upstream` skill, `.agents/fork-sync-policy.toml`, `docs/maintainers/fork-sync-policy.md`, and `docs/maintainers/fork-divergences.md`. - Merge method: `git merge --no-ff upstream/main` on `sync/upstream-2026-05-02-followup`. - Baseline update: `docs/maintainers/fork-divergences.md` now records upstream baseline `55e11db5924cf9b2c17a5c3fe63f071db6cab0a1` by immutable commit identity. ## Contract Review - Preserved local app/package identity as `codex-app`. - Preserved updater crate, binary, service, state, cache, and logs as `codex-app-updater`. - Preserved native package layout under `/opt/codex-app`, `/usr/lib/codex-app`, `/usr/bin`, and `/usr/share`. - Preserved native package versioning from the OpenAI DMG app bundle metadata. - Preserved the unprivileged updater boundary; privileged escalation remains install-time only. - Adapted upstream update bridge, launcher hooks, desktop actions, rebuild helpers, and tests to the local names and package layout. - Retained `codex-desktop` and `codex-update-manager.service` references only where they are migration or package compatibility metadata. - Preserved inspect mode as a non-installing flow by running inspect-only dependency checks and `inspect_rebuild_candidate` before install-dir metadata writes. ## Verification - `git merge-base --is-ancestor upstream/main HEAD` - `git diff --check` - `bash -n install.sh launcher/start.sh.template scripts/rebuild-candidate.sh scripts/lib/asar-patch.sh scripts/lib/install-helpers.sh scripts/lib/package-common.sh scripts/lib/rebuild-report.sh tests/scripts_smoke.sh` - `node --check scripts/lib/linux-update-bridge-patch.js` - `node --check scripts/lib/patch-report.js` - `node --check scripts/patch-linux-window-ui.js` - `cargo fmt --check --manifest-path updater/Cargo.toml` - `node --test scripts/patch-linux-window-ui.test.js` - `bash tests/scripts_smoke.sh` - `make check` - `cargo test --manifest-path computer-use-linux/Cargo.toml` - `cargo test -p codex-app-updater app::tests::install_ready_waits_when_app_is_running` - `make test` - `stat -c '%n %Y %y' Codex.dmg` -> `Codex.dmg 1777650917 2026-05-01 11:55:17.321165220 -0400` - `date +%s` -> `1777716699`; cached DMG was about 18h16m old and inside the 24h freshness gate. - `make build-app` - ASAR inspection verified the Linux quit-in-progress guard markers in `codex-app/resources/app.asar`. - `make pacman` -> built `dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst` and inspected package metadata. ## Review Fix Verification - `bash -n install.sh tests/scripts_smoke.sh` - `bash -n scripts/rebuild-candidate.sh tests/scripts_smoke.sh install.sh scripts/lib/package-common.sh` - `node --check scripts/lib/linux-update-bridge-patch.js` - `node --check scripts/patch-linux-window-ui.js` - `node --check scripts/patch-linux-window-ui.test.js` - `cargo fmt --manifest-path updater/Cargo.toml --check` - `node --test scripts/patch-linux-window-ui.test.js` - `bash tests/scripts_smoke.sh` - `cargo test -p codex-app-updater install_ready_marks` - `git diff --check` - `make check` - `make test` - `stat -c '%n %Y %y' Codex.dmg` -> `Codex.dmg 1777650917 2026-05-01 11:55:17.321165220 -0400` - `date +%s` -> `1777717688`; cached DMG was inside the 24h freshness gate. - `make build-app` - `make pacman` -> built `dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst`. - `bsdtar -xOf dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst usr/share/applications/codex-app.desktop` verified `/usr/bin/codex-app-updater check-now` and `/usr/bin/codex-app-updater install-ready`. - Final test-only review update: `bash -n tests/scripts_smoke.sh` and `bash tests/scripts_smoke.sh`. - Review closeout update at `ef365e23ceca0caef7898ce65f0d7c2cdaf8d353`: - `git diff --check` - `bash -n install.sh scripts/lib/install-helpers.sh scripts/lib/rebuild-report.sh scripts/rebuild-candidate.sh tests/scripts_smoke.sh` - `node --check scripts/lib/linux-update-bridge-patch.js` - `node --check scripts/patch-linux-window-ui.test.js` - `cargo fmt --manifest-path updater/Cargo.toml --check` - `node --test scripts/patch-linux-window-ui.test.js` - `bash tests/scripts_smoke.sh` - `make check` - `make test` - `stat -c '%n %Y %y' Codex.dmg` -> `Codex.dmg 1777650917 2026-05-01 11:55:17.321165220 -0400` - `date +%s` -> `1777718808`; cached DMG was inside the 24h freshness gate. - `make build-app` - `make pacman` -> built `dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst`. - `bsdtar -xOf dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst usr/share/applications/codex-app.desktop` verified `/usr/bin/codex-app-updater check-now` and `/usr/bin/codex-app-updater install-ready`. - Final review closeout update at `b588ce09f10383fa9b23d5582724b27ef1e8ae01`: - `git diff --check` - `bash -n scripts/lib/rebuild-report.sh tests/scripts_smoke.sh` - `node --check scripts/lib/linux-update-bridge-patch.js` - `node --check scripts/patch-linux-window-ui.test.js` - `cargo fmt --manifest-path updater/Cargo.toml --check` - `node --test scripts/patch-linux-window-ui.test.js` - `bash tests/scripts_smoke.sh` - `cargo test -p codex-app-updater install_ready_reports_unrecoverable_interrupted_install` - `make check` - `make test` - `stat -c '%n %Y %y' Codex.dmg` -> `Codex.dmg 1777650917 2026-05-01 11:55:17.321165220 -0400` - `date +%s` -> `1777719418`; cached DMG was inside the 24h freshness gate. - `make build-app` - `make pacman` -> built `dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst`. - `bsdtar -xOf dist/codex-app-26.429.20946-1-x86_64.pkg.tar.zst usr/share/applications/codex-app.desktop` verified `/usr/bin/codex-app-updater check-now` and `/usr/bin/codex-app-updater install-ready`. ## Unresolved Uncertainty None known. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Native Linux app-updater integration with desktop menu actions and a bridged updater flow * New "install-ready" command to apply prepared update packages * Side-by-side rebuild candidate workflow and an inspect-only inspection mode * **Improvements** * Structured JSON patch and rebuild reporting for diagnostics * Rebuild/install orchestration with safer atomic replacement and backups * Installer/CLI helper enhancements and new rebuild-oriented targets * **Tests** * Expanded coverage for updater flows, patching, inspect/report behavior * **Chores** * Updated ignore patterns to exclude rebuild artifacts and next-candidate output <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Sync Ledger
Contract Review
Verification
Notes
Uncertainty
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores