mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
dev
5 Commits
-
chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
## Why `argument-comment-lint` was green in CI even though the repo still had many uncommented literal arguments. The main gap was target coverage: the repo wrapper did not force Cargo to inspect test-only call sites, so examples like the `latest_session_lookup_params(true, ...)` tests in `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path. This change cleans up the existing backlog, makes the default repo lint path cover all Cargo targets, and starts rolling that stricter CI enforcement out on the platform where it is currently validated. ## What changed - mechanically fixed existing `argument-comment-lint` violations across the `codex-rs` workspace, including tests, examples, and benches - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to `--all-targets` unless the caller explicitly narrows the target set - fixed both wrappers so forwarded cargo arguments after `--` are preserved with a single separator - documented the new default behavior in `tools/argument-comment-lint/README.md` - updated `rust-ci` so the macOS lint lane keeps the plain wrapper invocation and therefore enforces `--all-targets`, while Linux and Windows temporarily pass `-- --lib --bins` That temporary CI split keeps the stricter all-targets check where it is already cleaned up, while leaving room to finish the remaining Linux- and Windows-specific target-gated cleanup before enabling `--all-targets` on those runners. The Linux and Windows failures on the intermediate revision were caused by the wrapper forwarding bug, not by additional lint findings in those lanes. ## Validation - `bash -n tools/argument-comment-lint/run.sh` - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh` - shell-level wrapper forwarding check for `-- --lib --bins` - shell-level wrapper forwarding check for `-- --tests` - `just argument-comment-lint` - `cargo test` in `tools/argument-comment-lint` - `cargo test -p codex-terminal-detection` ## Follow-up - Clean up remaining Linux-only target-gated callsites, then switch the Linux lint lane back to the plain wrapper invocation. - Clean up remaining Windows-only target-gated callsites, then switch the Windows lint lane back to the plain wrapper invocation.
Michael Bolin ·
2026-03-27 19:00:44 -07:00 -
jif-oai ·
2026-03-04 10:17:29 +00:00 -
feat(sleep-inhibitor): add Linux and Windows idle-sleep prevention (#11766)
## Background - follow-up to previous macOS-only PR: https://github.com/openai/codex/pull/11711 - follow-up macOS refactor PR (current structural approach used here): https://github.com/openai/codex/pull/12340 ## Summary - extend `codex-utils-sleep-inhibitor` with Linux and Windows backends while preserving existing macOS behavior - Linux backend: - use `systemd-inhibit` (`--what=idle --mode=block`) when available - fall back to `gnome-session-inhibit` (`--inhibit idle`) when available - keep no-op behavior if neither backend exists on host - Windows backend: - use Win32 power request handles (`PowerCreateRequest` + `PowerSetRequest` / `PowerClearRequest`) with `PowerRequestSystemRequired` - make `prevent_idle_sleep` Experimental on macOS/Linux/Windows; keep under development on other targets ## Testing - `just fmt` - `cargo test -p codex-utils-sleep-inhibitor` - `cargo test -p codex-core features::tests::` - `cargo test -p codex-tui chatwidget::tests::` - `just fix -p codex-utils-sleep-inhibitor` - `just fix -p codex-core` ## Semantics and API references - Goal remains: prevent idle system sleep while a turn is running. - Linux: - `systemd-inhibit` / login1 inhibitor model: - https://www.freedesktop.org/software/systemd/man/latest/systemd-inhibit.html - https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html - https://systemd.io/INHIBITOR_LOCKS/ - xdg-desktop-portal Inhibit (relevant for sandboxed apps): - https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Inhibit.html - Windows: - `PowerCreateRequest`: - https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-powercreaterequest - `PowerSetRequest`: - https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-powersetrequest - `PowerClearRequest`: - https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-powerclearrequest - `SetThreadExecutionState` (alternative baseline API): - https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setthreadexecutionstate ## Chromium vs this PR - Chromium Linux backend: - https://github.com/chromium/chromium/blob/main/services/device/wake_lock/power_save_blocker/power_save_blocker_linux.cc - Chromium Windows backend: - https://github.com/chromium/chromium/blob/main/services/device/wake_lock/power_save_blocker/power_save_blocker_win.cc - Electron powerSaveBlocker entry point: - https://github.com/electron/electron/blob/main/shell/browser/api/electron_api_power_save_blocker.cc ## Why we differ from Chromium - Linux implementation mechanism: - Chromium uses in-process D-Bus APIs plus UI-integrated screen-saver suspension. - This PR uses command-based inhibitor backends (`systemd-inhibit`, `gnome-session-inhibit`) instead of linking a Linux D-Bus client in this crate. - Reason: keep `codex-utils-sleep-inhibitor` dependency-light and avoid Linux CI/toolchain fragility from new native D-Bus linkage, while preserving the same runtime intent (hold an inhibitor while a turn runs). - Linux UI integration scope: - Chromium also uses `display::Screen::SuspendScreenSaver()` in its UI stack. - Codex `codex-rs` does not have that display abstraction in this crate, so this PR scopes Linux behavior to process-level sleep inhibition only. - Windows wake-lock type breadth: - Chromium supports both display/system wake-lock types and extra display-specific handling for some pre-Win11 scenarios. - Codex’s feature is scoped to turn execution continuity (not forcing display on), so this PR uses `PowerRequestSystemRequired` only.
Yaroslav Volovich ·
2026-02-24 11:51:44 -08:00 -
fix: simplify macOS sleep inhibitor FFI (#12340)
Summary - simplify the macOS sleep inhibitor FFI by replacing `dlopen` / `dlsym` / `transmute` with normal IOKit extern calls and `SAFETY` comments - switch to cfg-selected platform implementations (`imp::SleepInhibitor`) instead of `Box<dyn ...>` - check in minimal IOKit bindings generated with `bindgen` and include them from the macOS backend - enable direct IOKit linkage in Bazel macOS builds by registering `IOKit` in the Bazel `osx.framework(...)` toolchain extension list - update `Cargo.lock` and `MODULE.bazel.lock` after removing the build-time `bindgen` dependency path Testing - `just fmt` - `cargo clippy -p codex-utils-sleep-inhibitor --all-targets -- -D warnings` - `cargo test -p codex-utils-sleep-inhibitor` - `bazel test //codex-rs/utils/sleep-inhibitor:all --test_output=errors` - `just bazel-lock-update` - `just bazel-lock-check` Context - follow-up to #11711 addressing Ryan's review comments - `bindgen` is used to generate the checked-in bindings file, but not at build time
2026-02-20 09:52:21 -08:00 -
feat(tui): prevent macOS idle sleep while turns run (#11711)
## Summary - add a shared `codex-core` sleep inhibitor that uses native macOS IOKit assertions (`IOPMAssertionCreateWithName` / `IOPMAssertionRelease`) instead of spawning `caffeinate` - wire sleep inhibition to turn lifecycle in `tui` (`TurnStarted` enables; `TurnComplete` and abort/error finalization disable) - gate this behavior behind a `/experimental` feature toggle (`[features].prevent_idle_sleep`) instead of a dedicated `[tui]` config flag - expose the toggle in `/experimental` on macOS; keep it under development on other platforms - keep behavior no-op on non-macOS targets <img width="1326" height="577" alt="image" src="https://github.com/user-attachments/assets/73fac06b-97ae-46a2-800a-30f9516cf8a3" /> ## Testing - `cargo check -p codex-core -p codex-tui` - `cargo test -p codex-core sleep_inhibitor::tests -- --nocapture` - `cargo test -p codex-core tui_config_missing_notifications_field_defaults_to_enabled -- --nocapture` - `cargo test -p codex-core prevent_idle_sleep_is_ -- --nocapture` ## Semantics and API references - This PR targets `caffeinate -i` semantics: prevent *idle system sleep* while allowing display idle sleep. - `caffeinate -i` mapping in Apple open source (`assertionMap`): - `kIdleAssertionFlag -> kIOPMAssertionTypePreventUserIdleSystemSleep` - Source: https://github.com/apple-oss-distributions/PowerManagement/blob/PowerManagement-1846.60.12/caffeinate/caffeinate.c#L52-L54 - Apple IOKit docs for assertion types and API: - https://developer.apple.com/documentation/iokit/iopmlib_h/iopmassertiontypes - https://developer.apple.com/documentation/iokit/1557092-iopmassertioncreatewithname - https://developer.apple.com/library/archive/qa/qa1340/_index.html ## Codex Electron vs this PR (full stack path) - Codex Electron app requests sleep blocking with `powerSaveBlocker.start("prevent-app-suspension")`: - https://github.com/openai/codex/blob/main/codex/codex-vscode/electron/src/electron-message-handler.ts - Electron maps that string to Chromium wake lock type `kPreventAppSuspension`: - https://github.com/electron/electron/blob/main/shell/browser/api/electron_api_power_save_blocker.cc - Chromium macOS backend maps wake lock types to IOKit assertion constants and calls IOKit: - `kPreventAppSuspension -> kIOPMAssertionTypeNoIdleSleep` - `kPreventDisplaySleep / kPreventDisplaySleepAllowDimming -> kIOPMAssertionTypeNoDisplaySleep` - https://github.com/chromium/chromium/blob/main/services/device/wake_lock/power_save_blocker/power_save_blocker_mac.cc ## Why this PR uses a different macOS constant name - This PR uses `"PreventUserIdleSystemSleep"` directly, via `IOPMAssertionCreateWithName`, in `codex-rs/core/src/sleep_inhibitor.rs`. - Apple’s IOKit header documents `kIOPMAssertionTypeNoIdleSleep` as deprecated and recommends `kIOPMAssertPreventUserIdleSystemSleep` / `kIOPMAssertionTypePreventUserIdleSystemSleep`: - https://github.com/apple-oss-distributions/IOKitUser/blob/IOKitUser-100222.60.2/pwr_mgt.subproj/IOPMLib.h#L1000-L1030 - So Chromium and this PR are using different constant names, but semantically equivalent idle-system-sleep prevention behavior. ## Future platform support The architecture is intentionally set up for multi-platform extensions: - UI code (`tui`) only calls `SleepInhibitor::set_turn_running(...)` on turn lifecycle boundaries. - Platform-specific behavior is isolated in `codex-rs/core/src/sleep_inhibitor.rs` behind `cfg(...)` blocks. - Feature exposure is centralized in `core/src/features.rs` and surfaced via `/experimental`. - Adding new OS backends should not require additional TUI wiring; only the backend internals and feature stage metadata need to change. Potential follow-up implementations: - Windows: - Add a backend using Win32 power APIs (`SetThreadExecutionState(ES_CONTINUOUS | ES_SYSTEM_REQUIRED)` as baseline). - Optionally move to `PowerCreateRequest` / `PowerSetRequest` / `PowerClearRequest` for richer assertion semantics. - Linux: - Add a backend using logind inhibitors over D-Bus (`org.freedesktop.login1.Manager.Inhibit` with `what="sleep"`). - Keep a no-op fallback where logind/D-Bus is unavailable. This PR keeps the cross-platform API surface minimal so future PRs can add Windows/Linux support incrementally with low churn. --------- Co-authored-by: jif-oai <jif@openai.com>
2026-02-13 10:31:39 -08:00