mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
600c3e49e00f2effd4239964a01dd0ffbaa5727d
396 Commits
-
Codex/windows bazel rust test coverage no rs (#16528)
# Why this PR exists This PR is trying to fix a coverage gap in the Windows Bazel Rust test lane. Before this change, the Windows `bazel test //...` job was nominally part of PR CI, but a non-trivial set of `//codex-rs/...` Rust test targets did not actually contribute test signal on Windows. In particular, targets such as `//codex-rs/core:core-unit-tests`, `//codex-rs/core:core-all-test`, and `//codex-rs/login:login-unit-tests` were incompatible during Bazel analysis on the Windows gnullvm platform, so they never reached test execution there. That is why the Cargo-powered Windows CI job could surface Windows-only failures that the Bazel-powered job did not report: Cargo was executing those tests, while Bazel was silently dropping them from the runnable target set. The main goal of this PR is to make the Windows Bazel test lane execute those Rust test targets instead of skipping them during analysis, while still preserving `windows-gnullvm` as the target configuration for the code under test. In other words: use an MSVC host/exec toolchain where Bazel helper binaries and build scripts need it, but continue compiling the actual crate targets with the Windows gnullvm cfgs that our current Bazel matrix is supposed to exercise. # Important scope note This branch intentionally removes the non-resource-loading `.rs` test and production-code changes from the earlier `codex/windows-bazel-rust-test-coverage` branch. The only Rust source changes kept here are runfiles/resource-loading fixes in TUI tests: - `codex-rs/tui/src/chatwidget/tests.rs` - `codex-rs/tui/tests/manager_dependency_regression.rs` That is deliberate. Since the corresponding tests already pass under Cargo, this PR is meant to test whether Bazel infrastructure/toolchain fixes alone are enough to get a healthy Windows Bazel test signal, without changing test behavior for Windows timing, shell output, or SQLite file-locking. # How this PR changes the Windows Bazel setup ## 1. Split Windows host/exec and target concerns in the Bazel test lane The core change is that the Windows Bazel test job now opts into an MSVC host platform for Bazel execution-time tools, but only for `bazel test`, not for the Bazel clippy build. Files: - `.github/workflows/bazel.yml` - `.github/scripts/run-bazel-ci.sh` - `MODULE.bazel` What changed: - `run-bazel-ci.sh` now accepts `--windows-msvc-host-platform`. - When that flag is present on Windows, the wrapper appends `--host_platform=//:local_windows_msvc` unless the caller already provided an explicit `--host_platform`. - `bazel.yml` passes that wrapper flag only for the Windows `bazel test //...` job. - The Bazel clippy job intentionally does **not** pass that flag, so clippy stays on the default Windows gnullvm host/exec path and continues linting against the target cfgs we care about. - `run-bazel-ci.sh` also now forwards `CODEX_JS_REPL_NODE_PATH` on Windows and normalizes the `node` executable path with `cygpath -w`, so tests that need Node resolve the runner's Node installation correctly under the Windows Bazel test environment. Why this helps: - The original incompatibility chain was mostly on the **exec/tool** side of the graph, not in the Rust test code itself. Moving host tools to MSVC lets Bazel resolve helper binaries and generators that were not viable on the gnullvm exec platform. - Keeping the target platform on gnullvm preserves cfg coverage for the crates under test, which is important because some Windows behavior differs between `msvc` and `gnullvm`. ## 2. Teach the repo's Bazel Rust macro about Windows link flags and integration-test knobs Files: - `defs.bzl` - `codex-rs/core/BUILD.bazel` - `codex-rs/otel/BUILD.bazel` - `codex-rs/tui/BUILD.bazel` What changed: - Replaced the old gnullvm-only linker flag block with `WINDOWS_RUSTC_LINK_FLAGS`, which now handles both Windows ABIs: - gnullvm gets `-C link-arg=-Wl,--stack,8388608` - MSVC gets `-C link-arg=/STACK:8388608`, `-C link-arg=/NODEFAULTLIB:libucrt.lib`, and `-C link-arg=ucrt.lib` - Threaded those Windows link flags into generated `rust_binary`, unit-test binaries, and integration-test binaries. - Extended `codex_rust_crate(...)` with: - `integration_test_args` - `integration_test_timeout` - Used those new knobs to: - mark `//codex-rs/core:core-all-test` as a long-running integration test - serialize `//codex-rs/otel:otel-all-test` with `--test-threads=1` - Added `src/**/*.rs` to `codex-rs/tui` test runfiles, because one regression test scans source files at runtime and Bazel does not expose source-tree directories unless they are declared as data. Why this helps: - Once host-side MSVC tools are available, we still need the generated Rust test binaries to link correctly on Windows. The MSVC-side stack/UCRT flags make those binaries behave more like their Cargo-built equivalents. - The integration-test macro knobs avoid hardcoding one-off test behavior in ad hoc BUILD rules and make the generated test targets more expressive where Bazel and Cargo have different runtime defaults. ## 3. Patch `rules_rs` / `rules_rust` so Windows MSVC exec-side Rust and build scripts are actually usable Files: - `MODULE.bazel` - `patches/rules_rs_windows_exec_linker.patch` - `patches/rules_rust_windows_bootstrap_process_wrapper_linker.patch` - `patches/rules_rust_windows_build_script_runner_paths.patch` - `patches/rules_rust_windows_exec_msvc_build_script_env.patch` - `patches/rules_rust_windows_msvc_direct_link_args.patch` - `patches/rules_rust_windows_process_wrapper_skip_temp_outputs.patch` - `patches/BUILD.bazel` What these patches do: - `rules_rs_windows_exec_linker.patch` - Adds a `rust-lld` filegroup for Windows Rust toolchain repos, symlinked to `lld-link.exe` from `PATH`. - Marks Windows toolchains as using a direct linker driver. - Supplies Windows stdlib link flags for both gnullvm and MSVC. - `rules_rust_windows_bootstrap_process_wrapper_linker.patch` - For Windows MSVC Rust targets, prefers the Rust toolchain linker over an inherited C++ linker path like `clang++`. - This specifically avoids the broken mixed-mode command line where rustc emits MSVC-style `/NOLOGO` / `/LIBPATH:` / `/OUT:` arguments but Bazel still invokes `clang++.exe`. - `rules_rust_windows_build_script_runner_paths.patch` - Normalizes forward-slash execroot-relative paths into Windows path separators before joining them on Windows. - Uses short Windows paths for `RUSTC`, `OUT_DIR`, and the build-script working directory to avoid path-length and quoting issues in third-party build scripts. - Exposes `RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER=1` to build scripts so crate-local patches can detect "this is running under Bazel's build-script runner". - Fixes the Windows runfiles cleanup filter so generated files with retained suffixes are actually retained. - `rules_rust_windows_exec_msvc_build_script_env.patch` - For exec-side Windows MSVC build scripts, stops force-injecting Bazel's `CC`, `CXX`, `LD`, `CFLAGS`, and `CXXFLAGS` when that would send GNU-flavored tool paths/flags into MSVC-oriented Cargo build scripts. - Rewrites or strips GNU-only `--sysroot`, MinGW include/library paths, stack-protector, and `_FORTIFY_SOURCE` flags on the MSVC exec path. - The practical effect is that build scripts can fall back to the Visual Studio toolchain environment already exported by CI instead of crashing inside Bazel's hermetic `clang.exe` setup. - `rules_rust_windows_msvc_direct_link_args.patch` - When using a direct linker on Windows, stops forwarding GNU driver flags such as `-L...` and `--sysroot=...` that `lld-link.exe` does not understand. - Passes non-`.lib` native artifacts as explicit `-Clink-arg=<path>` entries when needed. - Filters C++ runtime libraries to `.lib` artifacts on the Windows direct-driver path. - `rules_rust_windows_process_wrapper_skip_temp_outputs.patch` - Excludes transient `*.tmp*` and `*.rcgu.o` files from process-wrapper dependency search-path consolidation, so unstable compiler outputs do not get treated as real link search-path inputs. Why this helps: - The host-platform split alone was not enough. Once Bazel started analyzing/running previously incompatible Rust tests on Windows, the next failures were in toolchain plumbing: - MSVC-targeted Rust tests were being linked through `clang++` with MSVC-style arguments. - Cargo build scripts running under Bazel's Windows MSVC exec platform were handed Unix/GNU-flavored path and flag shapes. - Some generated paths were too long or had path-separator forms that third-party Windows build scripts did not tolerate. - These patches make that mixed Bazel/Cargo/Rust/MSVC path workable enough for the test lane to actually build and run the affected crates. ## 4. Patch third-party crate build scripts that were not robust under Bazel's Windows MSVC build-script path Files: - `MODULE.bazel` - `patches/aws-lc-sys_windows_msvc_prebuilt_nasm.patch` - `patches/ring_windows_msvc_include_dirs.patch` - `patches/zstd-sys_windows_msvc_include_dirs.patch` What changed: - `aws-lc-sys` - Detects Bazel's Windows MSVC build-script runner via `RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER` or a `bazel-out` manifest-dir path. - Uses `clang-cl` for Bazel Windows MSVC builds when no explicit `CC`/`CXX` is set. - Allows prebuilt NASM on the Bazel Windows MSVC path even when `nasm` is not available directly in the runner environment. - Avoids canonicalizing `CARGO_MANIFEST_DIR` in the Bazel Windows MSVC case, because that path may point into Bazel output/runfiles state where preserving the given path is more reliable than forcing a local filesystem canonicalization. - `ring` - Under the Bazel Windows MSVC build-script runner, copies the pregenerated source tree into `OUT_DIR` and uses that as the generated-source root. - Adds include paths needed by MSVC compilation for Fiat/curve25519/P-256 generated headers. - Rewrites a few relative includes in C sources so the added include directories are sufficient. - `zstd-sys` - Adds MSVC-only include directories for `compress`, `decompress`, and feature-gated dictionary/legacy/seekable sources. - Skips `-fvisibility=hidden` on MSVC targets, where that GCC/Clang-style flag is not the right mechanism. Why this helps: - After the `rules_rust` plumbing started running build scripts on the Windows MSVC exec path, some third-party crates still failed for crate-local reasons: wrong compiler choice, missing include directories, build-script assumptions about manifest paths, or Unix-only C compiler flags. - These crate patches address those crate-local assumptions so the larger toolchain change can actually reach first-party Rust test execution. ## 5. Keep the only `.rs` test changes to Bazel/Cargo runfiles parity Files: - `codex-rs/tui/src/chatwidget/tests.rs` - `codex-rs/tui/tests/manager_dependency_regression.rs` What changed: - Instead of asking `find_resource!` for a directory runfile like `src/chatwidget/snapshots` or `src`, these tests now resolve one known file runfile first and then walk to its parent directory. Why this helps: - Bazel runfiles are more reliable for explicitly declared files than for source-tree directories that happen to exist in a Cargo checkout. - This keeps the tests working under both Cargo and Bazel without changing their actual assertions. # What we tried before landing on this shape, and why those attempts did not work ## Attempt 1: Force `--host_platform=//:local_windows_msvc` for all Windows Bazel jobs This did make the previously incompatible test targets show up during analysis, but it also pushed the Bazel clippy job and some unrelated build actions onto the MSVC exec path. Why that was bad: - Windows clippy started running third-party Cargo build scripts with Bazel's MSVC exec settings and crashed in crates such as `tree-sitter` and `libsqlite3-sys`. - That was a regression in a job that was previously giving useful gnullvm-targeted lint signal. What this PR does instead: - The wrapper flag is opt-in, and `bazel.yml` uses it only for the Windows `bazel test` lane. - The clippy lane stays on the default Windows gnullvm host/exec configuration. ## Attempt 2: Broaden the `rules_rust` linker override to all Windows Rust actions This fixed the MSVC test-lane failure where normal `rust_test` targets were linked through `clang++` with MSVC-style arguments, but it broke the default gnullvm path. Why that was bad: - `@@rules_rs++rules_rust+rules_rust//util/process_wrapper:process_wrapper` on the gnullvm exec platform started linking with `lld-link.exe` and then failed to resolve MinGW-style libraries such as `-lkernel32`, `-luser32`, and `-lmingw32`. What this PR does instead: - The linker override is restricted to Windows MSVC targets only. - The gnullvm path keeps its original linker behavior, while MSVC uses the direct Windows linker. ## Attempt 3: Keep everything on pure Windows gnullvm and patch the V8 / Python incompatibility chain instead This would have preserved a single Windows ABI everywhere, but it is a much larger project than this PR. Why that was not the practical first step: - The original incompatibility chain ran through exec-side generators and helper tools, not only through crate code. - `third_party/v8` is already special-cased on Windows gnullvm because `rusty_v8` only publishes Windows prebuilts under MSVC names. - Fixing that path likely means deeper changes in V8/rules_python/rules_rust toolchain resolution and generator execution, not just one local CI flag. What this PR does instead: - Keep gnullvm for the target cfgs we want to exercise. - Move only the Windows test lane's host/exec platform to MSVC, then patch the build-script/linker boundary enough for that split configuration to work. ## Attempt 4: Validate compatibility with `bazel test --nobuild ...` This turned out to be a misleading local validation command. Why: - `bazel test --nobuild ...` can successfully analyze targets and then still exit 1 with "Couldn't start the build. Unable to run tests" because there are no runnable test actions after `--nobuild`. Better local check: ```powershell bazel build --nobuild --keep_going --host_platform=//:local_windows_msvc //codex-rs/login:login-unit-tests //codex-rs/core:core-unit-tests //codex-rs/core:core-all-test ``` # Which patches probably deserve upstream follow-up My rough take is that the `rules_rs` / `rules_rust` patches are the highest-value upstream candidates, because they are fixing generic Windows host/exec + MSVC direct-linker behavior rather than Codex-specific test logic. Strong upstream candidates: - `patches/rules_rs_windows_exec_linker.patch` - `patches/rules_rust_windows_bootstrap_process_wrapper_linker.patch` - `patches/rules_rust_windows_build_script_runner_paths.patch` - `patches/rules_rust_windows_exec_msvc_build_script_env.patch` - `patches/rules_rust_windows_msvc_direct_link_args.patch` - `patches/rules_rust_windows_process_wrapper_skip_temp_outputs.patch` Why these seem upstreamable: - They address general-purpose problems in the Windows MSVC exec path: - missing direct-linker exposure for Rust toolchains - wrong linker selection when rustc emits MSVC-style args - Windows path normalization/short-path issues in the build-script runner - forwarding GNU-flavored CC/link flags into MSVC Cargo build scripts - unstable temp outputs polluting process-wrapper search-path state Potentially upstreamable crate patches, but likely with more care: - `patches/zstd-sys_windows_msvc_include_dirs.patch` - `patches/ring_windows_msvc_include_dirs.patch` - `patches/aws-lc-sys_windows_msvc_prebuilt_nasm.patch` Notes on those: - The `zstd-sys` and `ring` include-path fixes look fairly generic for MSVC/Bazel build-script environments and may be straightforward to propose upstream after we confirm CI stability. - The `aws-lc-sys` patch is useful, but it includes a Bazel-specific environment probe and CI-specific compiler fallback behavior. That probably needs a cleaner upstream-facing shape before sending it out, so upstream maintainers are not forced to adopt Codex's exact CI assumptions. Probably not worth upstreaming as-is: - The repo-local Starlark/test target changes in `defs.bzl`, `codex-rs/*/BUILD.bazel`, and `.github/scripts/run-bazel-ci.sh` are mostly Codex-specific policy and CI wiring, not generic rules changes. # Validation notes for reviewers On this branch, I ran the following local checks after dropping the non-resource-loading Rust edits: ```powershell cargo test -p codex-tui just --shell 'C:\Program Files\Git\bin\bash.exe' --shell-arg -lc -- fix -p codex-tui python .\tools\argument-comment-lint\run-prebuilt-linter.py -p codex-tui just --shell 'C:\Program Files\Git\bin\bash.exe' --shell-arg -lc fmt ``` One local caveat: - `just argument-comment-lint` still fails on this Windows machine for an unrelated Bazel toolchain-resolution issue in `//codex-rs/exec:exec-all-test`, so I used the direct prebuilt linter for `codex-tui` as the local fallback. # Expected reviewer takeaway If this PR goes green, the important conclusion is that the Windows Bazel test coverage gap was primarily a Bazel host/exec toolchain problem, not a need to make the Rust tests themselves Windows-specific. That would be a strong signal that the deleted non-resource-loading Rust test edits from the earlier branch should stay out, and that future work should focus on upstreaming the generic `rules_rs` / `rules_rust` Windows fixes and reducing the crate-local patch surface.
Michael Bolin ·
2026-04-03 15:34:03 -07:00 -
remove temporary ownership re-exports (#16626)
Stacked on #16508. This removes the temporary `codex-core` / `codex-login` re-export shims from the ownership split and rewrites callsites to import directly from `codex-model-provider-info`, `codex-models-manager`, `codex-api`, `codex-protocol`, `codex-feedback`, and `codex-response-debug-context`. No behavior change intended; this is the mechanical import cleanup layer split out from the ownership move. --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-04-03 00:33:34 -07:00 -
[codex] Remove codex-core config type shim (#16529)
## Why This finishes the config-type move out of `codex-core` by removing the temporary compatibility shim in `codex_core::config::types`. Callers now depend on `codex-config` directly, which keeps these config model types owned by the config crate instead of re-expanding `codex-core` as a transitive API surface. ## What Changed - Removed the `codex-rs/core/src/config/types.rs` re-export shim and the `core::config::ApprovalsReviewer` re-export. - Updated `codex-core`, `codex-cli`, `codex-tui`, `codex-app-server`, `codex-mcp-server`, and `codex-linux-sandbox` call sites to import `codex_config::types` directly. - Added explicit `codex-config` dependencies to downstream crates that previously relied on the `codex-core` re-export. - Regenerated `codex-rs/core/config.schema.json` after updating the config docs path reference.
Michael Bolin ·
2026-04-02 01:19:44 -07:00 -
Fix TUI app-server permission profile conversions (#16284)
Addresses #16283 Problem: TUI app-server permission approvals could drop filesystem grants because request and response payloads were round-tripped through mismatched camelCase and snake_case JSON shapes. Solution: Replace the lossy JSON round-trips with typed app-server/core permission conversions so requested and granted permission profiles, including filesystem paths and scope, are preserved end to end.
Eric Traut ·
2026-04-01 22:00:27 -06:00 -
fix(guardian): make GuardianAssessmentEvent.action strongly typed (#16448)
## Description Previously the `action` field on `EventMsg::GuardianAssessment`, which describes what Guardian is reviewing, was typed as an arbitrary JSON blob. This PR cleans it up and defines a sum type representing all the various actions that Guardian can review. This is a breaking change (on purpose), which is fine because: - the Codex app / VSCE does not actually use `action` at the moment - the TUI code that consumes `action` is updated in this PR as well - rollout files that serialized old `EventMsg::GuardianAssessment` will just silently drop these guardian events - the contract is defined as unstable, so other clients have a fair warning :) This will make things much easier for followup Guardian work. ## Why The old guardian review payloads worked, but they pushed too much shape knowledge into downstream consumers. The TUI had custom JSON parsing logic for commands, patches, network requests, and MCP calls, and the app-server protocol was effectively just passing through an opaque blob. Typing this at the protocol boundary makes the contract clearer.
Owen Lin ·
2026-04-01 15:42:18 -07:00 -
Fix stale /status rate limits in active TUI sessions (#16201)
Fix stale weekly limit in `/status` (#16194): /status reused the session’s cached rate-limit snapshot, so the weekly remaining limit could stay frozen within an active session. With this change, we now dynamically update the rate limits after status is displayed. I needed to delete a few low-value test cases from the chatWidget tests because the test.rs file is really large, and the new tests in this PR pushed us over the 512K mandated limit. I'm working on a separate PR to refactor that test file.
Eric Traut ·
2026-03-31 17:03:05 -06:00 -
Refactor chatwidget tests into topical modules (#16361)
Problem: `chatwidget/tests.rs` had grown into a single oversized test blob that was hard to maintain and exceeded the repo's blob size limit. Solution: split the chatwidget tests into topical modules with a thin root `tests.rs`, shared helper utilities, preserved snapshot naming, and hermetic test config so the refactor stays stable and passes the `codex-tui` test suite.
Eric Traut ·
2026-03-31 16:45:58 -06:00 -
Route TUI
/feedbacksubmission through the app server (#16184)The TUI’s `/feedback` flow was still uploading directly through the local feedback crate, which bypassed app-server behavior such as auth-derived feedback tags like chatgpt_user_id and made TUI feedback handling diverge from other clients. It also meant that remove TUI sessions failed to upload the correct feedback logs and session details. Testing: Manually tested `/feedback` flow and confirmed that it didn't regress.
Eric Traut ·
2026-03-31 10:36:47 -06:00 -
[codex] Normalize Windows path in MCP startup snapshot test (#16204)
## Summary A Windows-only snapshot assertion in the app-server MCP startup warning test compared the raw rendered path, so CI saw `C:\tmp\project` instead of the normalized `/tmp/project` snapshot fixture. ## Fix Route that snapshot assertion through the existing `normalize_snapshot_paths(...)` helper so the test remains platform-stable.
Eric Traut ·
2026-03-29 17:54:17 -06:00 -
Fix app-server TUI MCP startup warnings regression (#16041)
This addresses #16038 The default `tui_app_server` path stopped surfacing MCP startup failures during cold start, even though the legacy TUI still showed warnings like `MCP startup incomplete (...)`. The app-server bridge emitted per-server startup status notifications, but `tui_app_server` ignored them, so failed MCP handshakes could look like a clean startup. This change teaches `tui_app_server` to consume MCP startup status notifications, preserve the immediate per-server failure warning, and synthesize the same aggregate startup warning the legacy TUI shows once startup settles.
Eric Traut ·
2026-03-29 11:57:00 -06:00 -
Remove TUI voice transcription feature (#16114)
Removes the partially-completed TUI composer voice transcription flow, including its feature flag, app events, and hold-to-talk state machine.
Eric Traut ·
2026-03-29 00:20:25 +00:00 -
Rename tui_app_server to tui (#16104)
This is a follow-up to https://github.com/openai/codex/pull/15922. That previous PR deleted the old `tui` directory and left the new `tui_app_server` directory in place. This PR renames `tui_app_server` to `tui` and fixes up all references.
Eric Traut ·
2026-03-28 11:23:07 -06:00 -
Remove the legacy TUI split (#15922)
This is the part 1 of 2 PRs that will delete the `tui` / `tui_app_server` split. This part simply deletes the existing `tui` directory and marks the `tui_app_server` feature flag as removed. I left the `tui_app_server` feature flag in place for now so its presence doesn't result in an error. It is simply ignored. Part 2 will rename the `tui_app_server` directory `tui`. I did this as two parts to reduce visible code churn.
Eric Traut ·
2026-03-27 22:56:44 +00:00 -
chore: remove skill metadata from command approval payloads (#15906)
## Why This is effectively a follow-up to [#15812](https://github.com/openai/codex/pull/15812). That change removed the special skill-script exec path, but `skill_metadata` was still being threaded through command-approval payloads even though the approval flow no longer uses it to render prompts or resolve decisions. Keeping it around added extra protocol, schema, and client surface area without changing behavior. Removing it keeps the command-approval contract smaller and avoids carrying a dead field through app-server, TUI, and MCP boundaries. ## What changed - removed `ExecApprovalRequestSkillMetadata` and the corresponding `skillMetadata` field from core approval events and the v2 app-server protocol - removed the generated JSON and TypeScript schema output for that field - updated app-server, MCP server, TUI, and TUI app-server approval plumbing to stop forwarding the field - cleaned up tests that previously constructed or asserted `skillMetadata` ## Testing - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-protocol` - `cargo test -p codex-app-server-test-client` - `cargo test -p codex-mcp-server` - `just argument-comment-lint`
Michael Bolin ·
2026-03-26 15:32:03 -07:00 -
skills: remove unused skill permission metadata (#15900)
## Why Skill metadata accepted a `permissions` block and stored the result on `SkillMetadata`, but that data was never consumed by runtime behavior. Leaving the dead parsing path in place makes it look like skills can widen or otherwise influence execution permissions when, in practice, declared skill permissions are ignored. This change removes that misleading surface area so the skill metadata model matches what the system actually uses. ## What changed - removed `permission_profile` and `managed_network_override` from `core-skills::SkillMetadata` - stopped parsing `permissions` from skill metadata in `core-skills/src/loader.rs` - deleted the loader tests that only exercised the removed permissions parsing path - cleaned up dependent `SkillMetadata` constructors in tests and TUI code that were only carrying `None` for those fields ## Testing - `cargo test -p codex-core-skills` - `cargo test -p codex-tui submission_prefers_selected_duplicate_skill_path` - `just argument-comment-lint`
Michael Bolin ·
2026-03-26 19:33:23 +00:00 -
[hooks] add non-streaming (non-stdin style) shell-only PostToolUse support (#15531)
CHAINED PR - note that base is eternal/hooks-pretooluse-bash, not main -- so the following PR should be first Matching post-tool hook to the pre-tool functionality here: https://github.com/openai/codex/pull/15211 So, PreToolUse calls for plain shell calls, allows blocking. This PostToolUse call runs after the command executed example run: ``` › as a test, run in parallel the following commands: - echo 'one' - echo '[block-pre-tool-use]' - echo '[block-post-tool-use]' ⚠ MCP startup incomplete (failed: notion, linear) • Cruising through those three commands in parallel now, and I’ll share the exact outputs right after they land. • Running PreToolUse hook: checking the observatory runes • Running PreToolUse hook: checking the observatory runes • Running PreToolUse hook: checking the observatory runes PreToolUse hook (blocked) warning: wizard-tower PreToolUse demo blocked a Bash command on purpose. feedback: PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue. PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo 'one' PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo '[block-post-tool-use]' • Ran echo '[block-post-tool-use]' └ [block-post-tool-use] • Ran echo 'one' └ one • Running PostToolUse hook: reading the observatory aftermath • Running PostToolUse hook: reading the observatory aftermath PostToolUse hook (blocked) warning: wizard-tower PostToolUse demo blocked the Bash follow-through on purpose. feedback: PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to continue. PostToolUse hook (completed) warning: wizard-tower PostToolUse demo inspected Bash: echo 'one' hook context: PostToolUse demo saw the Bash result. Response preview: one ────────────────────────────────────────────────────────────────────────────────────────────────────── • Ran them in parallel. Little wave report: - echo 'one' completed and returned one. - echo '[block-pre-tool-use]' was blocked before execution by the PreToolUse hook. - echo '[block-post-tool-use]' ran, but its result was blocked afterward by the PostToolUse hook. The hook messages were: - PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue. - PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to continue. › for the blocked messages (correctly blocked in this test) did the tool output land back into context? • Yeah, from this run: - PreToolUse block: the command itself did not execute, so no command output landed in context. Only the block message did. - PostToolUse block: the command did execute, but the actual stdout did not come back into context. Only the post-tool block message did. So the blocked notifications landed in context, but the blocked command payloads themselves did not. Nice clean guardrail, aloha. ```
Andrei Eternal ·
2026-03-25 19:18:03 -07:00 -
Use AbsolutePathBuf for cwd state (#15710)
Migrate `cwd` and related session/config state to `AbsolutePathBuf` so downstream consumers consistently see absolute working directories. Add test-only `.abs()` helpers for `Path`, `PathBuf`, and `TempDir`, and update branch-local tests to use them instead of `AbsolutePathBuf::try_from(...)`. For the remaining TUI/app-server snapshot coverage that renders absolute cwd values, keep the snapshots unchanged and skip the Windows-only cases where the platform-specific absolute path layout differs.
pakrym-oai ·
2026-03-25 16:02:22 +00:00 -
Add legal link to TUI /plugin details (#15692)
- Adds language and "[learn more](https://help.openai.com/en/articles/11487775-apps-in-chatgpt)" link to plugin details pages. - Message is hidden when plugin is installed <img width="1970" height="498" alt="image" src="https://github.com/user-attachments/assets/f14330f7-661e-4860-8538-6dc9e8bbd90a" />
canvrno-oai ·
2026-03-24 17:40:26 -07:00 -
Clean up TUI /plugins row allignment (#15669)
- Remove marketplace from left column. - Change `Can be installed` to `Available` - Align right-column marketplace + selected-row hint text across states. - Changes applied to both `tui` and `tui_app_server`. - Update related snapshots/tests. <img width="2142" height="590" alt="image" src="https://github.com/user-attachments/assets/6e60b783-2bea-46d4-b353-f2fd328ac4d0" />
canvrno-oai ·
2026-03-24 20:27:10 +00:00 -
Move git utilities into a dedicated crate (#15564)
- create `codex-git-utils` and move the shared git helpers into it with file moves preserved for diff readability - move the `GitInfo` helpers out of `core` so stacked rollout work can depend on the shared crate without carrying its own git info module --------- Co-authored-by: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-24 13:26:23 -07:00 -
xl-openai ·
2026-03-24 11:47:23 -07:00 -
[Codex TUI] - Sort /plugins TUI menu by installed status first, alpha second (#15558)
Updates plugin ordering so installed plugins are listed first, with alphabetical sorting applied within the installed and uninstalled groups. The behavior is now consistent across both `tui` and `tui_app_server`, and related tests/snapshots were updated.
canvrno-oai ·
2026-03-24 09:35:52 -07:00 -
Remove filter from plugins/list result (#15580)
Show all plugin marketplaces in the /plugins popup by removing the `openai-curated` marketplace filter, and update plugin popup copy/tests/snapshots to match the new behavior in both TUI codepaths.
canvrno-oai ·
2026-03-23 23:41:01 -07:00 -
[hooks] add non-streaming (non-stdin style) shell-only PreToolUse support (#15211)
- add `PreToolUse` hook for bash-like tool execution only at first - block shell execution before dispatch with deny-only hook behavior - introduces common.rs matcher framework for matching when hooks are run example run: ``` › run three parallel echo commands, and the second one should echo "[block-pre-tool-use]" as a test • Running the three echo commands in parallel now and I’ll report the output directly. • Running PreToolUse hook: name for demo pre tool use hook • Running PreToolUse hook: name for demo pre tool use hook • Running PreToolUse hook: name for demo pre tool use hook PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo "first parallel echo" PreToolUse hook (blocked) warning: wizard-tower PreToolUse demo blocked a Bash command on purpose. feedback: PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue. PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo "third parallel echo" • Ran echo "first parallel echo" └ first parallel echo • Ran echo "third parallel echo" └ third parallel echo • Three little waves went out in parallel. 1. printed first parallel echo 2. was blocked before execution because it contained the exact test string [block-pre-tool-use] 3. printed third parallel echo There was also an unrelated macOS defaults warning around the successful commands, but the echoes themselves worked fine. If you want, I can rerun the second one with a slightly modified string so it passes cleanly. ```
Andrei Eternal ·
2026-03-23 14:32:59 -07:00 -
jif-oai ·
2026-03-23 20:51:07 +00:00 -
Unify realtime stop handling in TUI (#15529)
## Summary - route /realtime, Ctrl+C, and deleted realtime meters through the same realtime stop path - keep generic transcription placeholder cleanup free of realtime shutdown side effects ## Testing - Ran - Relied on CI for verification; did not run local tests --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-23 13:47:33 -07:00 -
Plugins TUI install/uninstall (#15342)
- Add install/uninstall actions to the TUI plugins menu - Wire plugin install/uninstall through both TUI and `tui_app_server` - Refresh config/plugin state after changes so the UI updates immediately - Add a post-install app setup flow for plugins that require additional app auth <img width="1567" height="300" alt="Screenshot 2026-03-20 at 4 08 44 PM" src="https://github.com/user-attachments/assets/366bd31b-2ffd-4e80-b4a3-3a9a9c674a5f" /> <img width="445" height="240" alt="Screenshot 2026-03-20 at 4 08 54 PM" src="https://github.com/user-attachments/assets/613999ab-269a-4758-ab59-7c057a1742dc" /> <img width="797" height="219" alt="Screenshot 2026-03-20 at 4 09 07 PM" src="https://github.com/user-attachments/assets/b9679e60-40f5-49bb-ade0-2e40449c3fbf" /> <img width="499" height="235" alt="Screenshot 2026-03-20 at 4 09 24 PM" src="https://github.com/user-attachments/assets/261ce2fe-f356-4e99-8ac9-f29ed850bc75" /> Note/known issue: The /plugin install flow fails in `tui_app_server` because after a successful install it tries to trigger a ReloadUserConfig operation, but `tui_app_server` has not yet implemented transport for that operation, so it falls through to the generic “Not available in app-server TUI yet” stub.
canvrno-oai ·
2026-03-23 12:38:39 -07:00 -
tui: queue follow-ups during manual /compact (#15259)
## Summary - queue input after the user submits `/compact` until that manual compact turn ends - mirror the same behavior in the app-server TUI - add regressions for input queued before compact starts and while it is running Co-authored-by: Codex <noreply@openai.com>
Charley Cunningham ·
2026-03-23 10:19:44 -07:00 -
Use Shift+Left to edit queued messages in tmux (#15480)
## Summary - use Shift+Left to edit the most recent queued message when running under tmux - mirror the same binding change in the app-server TUI - add tmux-specific tests and snapshot coverage for the rendered queued-message hint ## Testing - just fmt - cargo test -p codex-tui - cargo test -p codex-tui-app-server - just argument-comment-lint -p codex-tui -p codex-tui-app-server Co-authored-by: Codex <noreply@openai.com>
Charley Cunningham ·
2026-03-22 21:19:31 -07:00 -
[apps] Improve app tools loading for TUI. (#15376)
- [x] Remove the app tools copy in TUI and reference the core tools instead, this reduces tools/list calls from 4 to just 1.
Matthew Zeng ·
2026-03-22 00:17:48 -07:00 -
Initial plugins TUI menu - list and read only. tui + tui_app_server (#15215)
### Preliminary /plugins TUI menu - Adds a preliminary /plugins menu flow in both tui and tui_app_server. - Fetches plugin list data asynchronously and shows loading/error/cached states. - Limits this first pass to the curated ChatGPT marketplace. - Shows available plugins with installed/status metadata. - Supports in-menu search over plugin display name, plugin id, plugin name, and marketplace label. - Opens a plugin detail view on selection, including summaries for Skills, Apps, and MCP Servers, with back navigation. ### Testing - Launch codex-cli with plugins enabled (`--enable plugins`). - Run /plugins and verify: - loading state appears first - plugin list is shown - search filters results - selecting a plugin opens detail view, with a list of skills/connectors/MCP servers for the plugin - back action returns to the list. - Verify disabled behavior by running /plugins without plugins enabled (shows “Plugins are disabled” message). - Launch with `--enable tui_app_server` (and plugins enabled) and repeat the same /plugins flow; behavior should match.canvrno-oai ·
2026-03-19 21:28:33 -07:00 -
Use released DotSlash package for argument-comment lint (#15199)
## Why The argument-comment lint now has a packaged DotSlash artifact from [#15198](https://github.com/openai/codex/pull/15198), so the normal repo lint path should use that released payload instead of rebuilding the lint from source every time. That keeps `just clippy` and CI aligned with the shipped artifact while preserving a separate source-build path for people actively hacking on the lint crate. The current alpha package also exposed two integration wrinkles that the repo-side prebuilt wrapper needs to smooth over: - the bundled Dylint library filename includes the host triple, for example `@nightly-2025-09-18-aarch64-apple-darwin`, and Dylint derives `RUSTUP_TOOLCHAIN` from that filename - on Windows, Dylint's driver path also expects `RUSTUP_HOME` to be present in the environment Without those adjustments, the prebuilt CI jobs fail during `cargo metadata` or driver setup. This change makes the checked-in prebuilt wrapper normalize the packaged library name to the plain `nightly-2025-09-18` channel before invoking `cargo-dylint`, and it teaches both the wrapper and the packaged runner source to infer `RUSTUP_HOME` from `rustup show home` when the environment does not already provide it. After the prebuilt Windows lint job started running successfully, it also surfaced a handful of existing anonymous literal callsites in `windows-sandbox-rs`. This PR now annotates those callsites so the new cross-platform lint job is green on the current tree. ## What Changed - checked in the current `tools/argument-comment-lint/argument-comment-lint` DotSlash manifest - kept `tools/argument-comment-lint/run.sh` as the source-build wrapper for lint development - added `tools/argument-comment-lint/run-prebuilt-linter.sh` as the normal enforcement path, using the checked-in DotSlash package and bundled `cargo-dylint` - updated `just clippy` and `just argument-comment-lint` to use the prebuilt wrapper - split `.github/workflows/rust-ci.yml` so source-package checks live in a dedicated `argument_comment_lint_package` job, while the released lint runs in an `argument_comment_lint_prebuilt` matrix on Linux, macOS, and Windows - kept the pinned `nightly-2025-09-18` toolchain install in the prebuilt CI matrix, since the prebuilt package still relies on rustup-provided toolchain components - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` to normalize host-qualified nightly library filenames, keep the `rustup` shim directory ahead of direct toolchain `cargo` binaries, and export `RUSTUP_HOME` when needed for Windows Dylint driver setup - updated `tools/argument-comment-lint/src/bin/argument-comment-lint.rs` so future published DotSlash artifacts apply the same nightly-filename normalization and `RUSTUP_HOME` inference internally - fixed the remaining Windows lint violations in `codex-rs/windows-sandbox-rs` by adding the required `/*param*/` comments at the reported callsites - documented the checked-in DotSlash file, wrapper split, archive layout, nightly prerequisite, and Windows `RUSTUP_HOME` requirement in `tools/argument-comment-lint/README.md`
Michael Bolin ·
2026-03-20 03:19:22 +00:00 -
Split features into codex-features crate (#15253)
- Split the feature system into a new `codex-features` crate. - Cut `codex-core` and workspace consumers over to the new config and warning APIs. Co-authored-by: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-19 20:12:07 -07:00 -
adding full imagepath to tui (#15154)
adding full path to TUI so image is open-able in the TUI after being generated. LImited to VSCode Terminal for now.
Won Park ·
2026-03-19 21:29:22 +00:00 -
Move terminal module to terminal-detection crate (#15216)
- Move core/src/terminal.rs and its tests into a standalone terminal-detection workspace crate. - Update direct consumers to depend on codex-terminal-detection and import terminal APIs directly. --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-19 14:08:04 -07:00 -
feat(tui): add /title terminal title configuration (#12334)
## Problem When multiple Codex sessions are open at once, terminal tabs and windows are hard to distinguish from each other. The existing status line only helps once the TUI is already focused, so it does not solve the "which tab is this?" problem. This PR adds a first-class `/title` command so the terminal window or tab title can carry a short, configurable summary of the current session. ## Screenshot <img width="849" height="320" alt="image" src="https://github.com/user-attachments/assets/8b112927-7890-45ed-bb1e-adf2f584663d" /> ## Mental model `/statusline` and `/title` are separate status surfaces with different constraints. The status line is an in-app footer that can be denser and more detailed. The terminal title is external terminal metadata, so it needs short, stable segments that still make multiple sessions easy to tell apart. The `/title` configuration is an ordered list of compact items. By default it renders `spinner,project`, so active sessions show lightweight progress first while idle sessions still stay easy to disambiguate. Each configured item is omitted when its value is not currently available rather than forcing a placeholder. ## Non-goals This does not merge `/title` into `/statusline`, and it does not add an arbitrary free-form title string. The feature is intentionally limited to a small set of structured items so the title stays short and reviewable. This also does not attempt to restore whatever title the terminal or shell had before Codex started. When Codex clears the title, it clears the title Codex last wrote. ## Tradeoffs A separate `/title` command adds some conceptual overlap with `/statusline`, but it keeps title-specific constraints explicit instead of forcing the status line model to cover two different surfaces. Title refresh can happen frequently, so the implementation now shares parsing and git-branch orchestration between the status line and title paths, and caches the derived project-root name by cwd. That keeps the hot path cheap without introducing background polling. ## Architecture The TUI gets a new `/title` slash command and a dedicated picker UI for selecting and ordering terminal-title items. The chosen ids are persisted in `tui.terminal_title`, with `spinner` and `project` as the default when the config is unset. `status` remains available as a separate text item, so configurations like `spinner,status` render compact progress like `⠋ Working`. `ChatWidget` now refreshes both status surfaces through a shared `refresh_status_surfaces()` path. That shared path parses configured items once, warns on invalid ids once, synchronizes shared cached state such as git-branch lookup, then renders the footer status line and terminal title from the same snapshot. Low-level OSC title writes live in `codex-rs/tui/src/terminal_title.rs`, which owns the terminal write path and last-mile sanitization before emitting OSC 0. ## Security Terminal-title text is treated as untrusted display content before Codex emits it. The write path strips control characters, removes invisible and bidi formatting characters that can make the title visually misleading, normalizes whitespace, and caps the emitted length. References used while implementing this: - [xterm control sequences](https://invisible-island.net/xterm/ctlseqs/ctlseqs.html) - [WezTerm escape sequences](https://wezterm.org/escape-sequences.html) - [CWE-150: Improper Neutralization of Escape, Meta, or Control Sequences](https://cwe.mitre.org/data/definitions/150.html) - [CERT VU#999008 (Trojan Source)](https://kb.cert.org/vuls/id/999008) - [Trojan Source disclosure site](https://trojansource.codes/) - [Unicode Bidirectional Algorithm (UAX #9)](https://www.unicode.org/reports/tr9/) - [Unicode Security Considerations (UTR #36)](https://www.unicode.org/reports/tr36/) ## Observability Unknown configured title item ids are warned about once instead of repeatedly spamming the transcript. Live preview applies immediately while the `/title` picker is open, and cancel rolls the in-memory title selection back to the pre-picker value. If terminal title writes fail, the TUI emits debug logs around set and clear attempts. The rendered status label intentionally collapses richer internal states into compact title text such as `Starting...`, `Ready`, `Thinking...`, `Working...`, `Waiting...`, and `Undoing...` when `status` is configured. ## Tests Ran: - `just fmt` - `cargo test -p codex-tui` At the moment, the red Windows `rust-ci` failures are due to existing `codex-core` `apply_patch_cli` stack-overflow tests that also reproduce on `main`. The `/title`-specific `codex-tui` suite is green.
Yaroslav Volovich ·
2026-03-19 19:26:36 +00:00 -
feat: add memory citation to agent message (#14821)
Client side to come
jif-oai ·
2026-03-18 10:03:38 +00:00 -
Handle realtime conversation end in the TUI (#14903)
- close live realtime sessions on errors, ctrl-c, and active meter removal - centralize TUI realtime cleanup and avoid duplicate follow-up close info --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com>
Ahmed Ibrahim ·
2026-03-17 21:04:58 -07:00 -
Stabilize permissions popup selection tests (#14966)
## What is flaky The permissions popup tests in the TUI are flaky, especially on Windows. They assume the popup opens on a specific row and that a fixed number of `Up` or `Down` keypresses will land on a specific preset. They also match popup text too loosely, so a non-selected row can satisfy the assertion. ## Why it was flaky These tests were asserting incidental rendering details rather than the actual selected permission preset. On Windows, the initial selection can differ from non-Windows runs. Some tests also searched the entire popup for text like `Guardian Approvals` or `(current)`, which can match a row that is visible but not selected. Once the popup order or current preset shifted slightly, a test could fail even though the UI behavior was still correct. ## How this PR fixes it This PR adds helpers that identify the selected popup row and selected preset name directly. The tests now assert the current selection by name, navigate to concrete target presets instead of assuming a fixed number of keypresses, and explicitly set the reviewer state in the cases that require `Guardian Approvals` to be current. ## Why this fix fixes the flakiness The assertions now track semantic state, not fragile text placement. Navigation is target-based instead of order-based, so Windows/non-Windows row differences and harmless popup layout changes no longer break the tests. That removes the scheduler- and platform-sensitive assumptions that made the popup suite intermittent. Co-authored-by: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-17 20:45:44 +00:00 -
Reuse guardian session across approvals (#14668)
## Summary - reuse a guardian subagent session across approvals so reviews keep a stable prompt cache key and avoid one-shot startup overhead - clear the guardian child history before each review so prior guardian decisions do not leak into later approvals - include the `smart_approvals` -> `guardian_approval` feature flag rename in the same PR to minimize release latency on a very tight timeline - add regression coverage for prompt-cache-key reuse without prior-review prompt bleed ## Request - Bug/enhancement request: internal guardian prompt-cache and latency improvement request --------- Co-authored-by: Codex <noreply@openai.com>
Charley Cunningham ·
2026-03-15 22:56:18 -07:00 -
Preserve background terminals on interrupt and rename cleanup command to /stop (#14602)
### Motivation - Interrupting a running turn (Ctrl+C / Esc) currently also terminates long‑running background shells, which is surprising for workflows like local dev servers or file watchers. - The existing cleanup command name was confusing; callers expect an explicit command to stop background terminals rather than a UI clear action. - Make background‑shell termination explicit and surface a clearer command name while preserving backward compatibility. ### Description - Renamed the background‑terminal cleanup slash command from `Clean` (`/clean`) to `Stop` (`/stop`) and kept `clean` as an alias in the command parsing/visibility layer, updated the user descriptions and command popup wiring accordingly. - Updated the unified‑exec footer text and snapshots to point to `/stop` (and trimmed corresponding snapshot output to match the new label). - Changed interrupt behavior so `Op::Interrupt` (Ctrl+C / Esc interrupt) no longer closes or clears tracked unified exec / background terminal processes in the TUI or core cleanup path; background shells are now preserved after an interrupt. - Updated protocol/docs to clarify that `turn/interrupt` (or `Op::Interrupt`) interrupts the active turn but does not terminate background terminals, and that `thread/backgroundTerminals/clean` is the explicit API to stop those shells. - Updated unit/integration tests and insta snapshots in the TUI and core unified‑exec suites to reflect the new semantics and command name. ### Testing - Ran formatting with `just fmt` in `codex-rs` (succeeded). - Ran `cargo test -p codex-protocol` (succeeded). - Attempted `cargo test -p codex-tui` but the build could not complete in this environment due to a native build dependency that requires `libcap` development headers (the `codex-linux-sandbox` vendored build step); install `libcap-dev` / make `libcap.pc` available in `PKG_CONFIG_PATH` to run the TUI test suite locally. - Updated and accepted the affected `insta` snapshots for the TUI changes so visual diffs reflect the new `/stop` wording and preserved interrupt behavior. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_69b39c44b6dc8323bd133ae206310fae)
friel-openai ·
2026-03-15 22:17:25 -07:00 -
Fix Windows CI assertions for guardian and Smart Approvals (#14645)
- Normalize guardian assessment path serialization to use forward slashes for cross-platform stability. - Seed workspace-write defaults in the Smart Approvals override-turn-context test so Windows and non-Windows selection flows are consistent. --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Charles Cunningham <ccunningham@openai.com>
Ahmed Ibrahim ·
2026-03-14 02:15:58 +00:00 -
Add Smart Approvals guardian review across core, app-server, and TUI (#13860)
## Summary - add `approvals_reviewer = "user" | "guardian_subagent"` as the runtime control for who reviews approval requests - route Smart Approvals guardian review through core for command execution, file changes, managed-network approvals, MCP approvals, and delegated/subagent approval flows - expose guardian review in app-server with temporary unstable `item/autoApprovalReview/{started,completed}` notifications carrying `targetItemId`, `review`, and `action` - update the TUI so Smart Approvals can be enabled from `/experimental`, aligned with the matching `/approvals` mode, and surfaced clearly while reviews are pending or resolved ## Runtime model This PR does not introduce a new `approval_policy`. Instead: - `approval_policy` still controls when approval is needed - `approvals_reviewer` controls who reviewable approval requests are routed to: - `user` - `guardian_subagent` `guardian_subagent` is a carefully prompted reviewer subagent that gathers relevant context and applies a risk-based decision framework before approving or denying the request. The `smart_approvals` feature flag is a rollout/UI gate. Core runtime behavior keys off `approvals_reviewer`. When Smart Approvals is enabled from the TUI, it also switches the current `/approvals` settings to the matching Smart Approvals mode so users immediately see guardian review in the active thread: - `approval_policy = on-request` - `approvals_reviewer = guardian_subagent` - `sandbox_mode = workspace-write` Users can still change `/approvals` afterward. Config-load behavior stays intentionally narrow: - plain `smart_approvals = true` in `config.toml` remains just the rollout/UI gate and does not auto-set `approvals_reviewer` - the deprecated `guardian_approval = true` alias migration does backfill `approvals_reviewer = "guardian_subagent"` in the same scope when that reviewer is not already configured there, so old configs preserve their original guardian-enabled behavior ARC remains a separate safety check. For MCP tool approvals, ARC escalations now flow into the configured reviewer instead of always bypassing guardian and forcing manual review. ## Config stability The runtime reviewer override is stable, but the config-backed app-server protocol shape is still settling. - `thread/start`, `thread/resume`, and `turn/start` keep stable `approvalsReviewer` overrides - the config-backed `approvals_reviewer` exposure returned via `config/read` (including profile-level config) is now marked `[UNSTABLE]` / experimental in the app-server protocol until we are more confident in that config surface ## App-server surface This PR intentionally keeps the guardian app-server shape narrow and temporary. It adds generic unstable lifecycle notifications: - `item/autoApprovalReview/started` - `item/autoApprovalReview/completed` with payloads of the form: - `{ threadId, turnId, targetItemId, review, action? }` `review` is currently: - `{ status, riskScore?, riskLevel?, rationale? }` - where `status` is one of `inProgress`, `approved`, `denied`, or `aborted` `action` carries the guardian action summary payload from core when available. This lets clients render temporary standalone pending-review UI, including parallel reviews, even when the underlying tool item has not been emitted yet. These notifications are explicitly documented as `[UNSTABLE]` and expected to change soon. This PR does **not** persist guardian review state onto `thread/read` tool items. The intended follow-up is to attach guardian review state to the reviewed tool item lifecycle instead, which would improve consistency with manual approvals and allow thread history / reconnect flows to replay guardian review state directly. ## TUI behavior - `/experimental` exposes the rollout gate as `Smart Approvals` - enabling it in the TUI enables the feature and switches the current session to the matching Smart Approvals `/approvals` mode - disabling it in the TUI clears the persisted `approvals_reviewer` override when appropriate and returns the session to default manual review when the effective reviewer changes - `/approvals` still exposes the reviewer choice directly - the TUI renders: - pending guardian review state in the live status footer, including parallel review aggregation - resolved approval/denial state in history ## Scope notes This PR includes the supporting core/runtime work needed to make Smart Approvals usable end-to-end: - shell / unified-exec / apply_patch / managed-network / MCP guardian review - delegated/subagent approval routing into guardian review - guardian review risk metadata and action summaries for app-server/TUI - config/profile/TUI handling for `smart_approvals`, `guardian_approval` alias migration, and `approvals_reviewer` - a small internal cleanup of delegated approval forwarding to dedupe fallback paths and simplify guardian-vs-parent approval waiting (no intended behavior change) Out of scope for this PR: - redesigning the existing manual approval protocol shapes - persisting guardian review state onto app-server `ThreadItem`s - delegated MCP elicitation auto-review (the current delegated MCP guardian shim only covers the legacy `RequestUserInput` path) --------- Co-authored-by: Codex <noreply@openai.com>Charley Cunningham ·
2026-03-13 15:27:00 -07:00 -
Override local apps settings with requirements.toml settings (#14304)
This PR changes app and connector enablement when `requirements.toml` is present locally or via remote configuration. For apps.* entries: - `enabled = false` in `requirements.toml` overrides the user’s local `config.toml` and forces the app to be disabled. - `enabled = true` in `requirements.toml` does not re-enable an app the user has disabled in config.toml. This behavior applies whether or not the user has an explicit entry for that app in `config.toml`. It also applies to cloud-managed policies and configurations when the admin sets the override through `requirements.toml`. Scenarios tested and verified: - Remote managed, user config (present) override - Admin-defined policies & configurations include a connector override: `[apps.<appID>] enabled = false` - User's config.toml has the same connector configured with `enabled = true` - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Remote managed, user config (absent) override - Admin-defined policies & configurations include a connector override: `[apps.<appID>] enabled = false` - User's config.toml has no entry for the the same connector - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Locally managed, user config (present) override - Local requirements.toml includes a connector override: `[apps.<appID>] enabled = false` - User's config.toml has the same connector configured with `enabled = true` - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer - Locally managed, user config (absent) override - Local requirements.toml includes a connector override: `[apps.<appID>] enabled = false` - User's config.toml has no entry for the the same connector - TUI/App should show connector as disabled - Connector should be unavailable for use in the composer <img width="1446" height="753" alt="image" src="https://github.com/user-attachments/assets/61c714ca-dcca-4952-8ad2-0afc16ff3835" /> <img width="595" height="233" alt="image" src="https://github.com/user-attachments/assets/7c8ab147-8fd7-429a-89fb-591c21c15621" />
canvrno-oai ·
2026-03-13 12:40:24 -07:00 -
Use subagents naming in the TUI (#14618)
- rename user-facing TUI multi-agent wording to subagents - rename the surfaced slash command to `subagents` and update tests/snapshots Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-13 19:08:38 +00:00 -
feat: support skill-scoped managed network domain overrides in skill config (#14522)
## Summary This lets skill loading split `permissions.network` into two distinct pieces: - `permissions.network.enabled` still feeds the skill `PermissionProfile` and remains the coarse gate for whether the skill can use network access at all. - `permissions.network.allowed_domains` and `permissions.network.denied_domains` are lifted into a new `SkillManagedNetworkOverride` so managed-network sessions can start per-skill scoped proxies with the right domain overrides. The change also updates `SkillMetadata` construction sites and adds loader tests covering YAML parsing plus normalization of the network gate vs. domain override fields. ## Follow-up A PR that uses the network_override to spin up a skill-specific proxy if network_override is not none.
Celia Chen ·
2026-03-13 04:45:14 +00:00 -
Include spawn agent model metadata in app-server items (#14410)
- add model and reasoning effort to app-server collab spawn items and notifications - regenerate app-server protocol schemas for the new fields --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-11 19:25:21 -07:00 -
Show spawned agent model and effort in TUI (#14273)
- include the requested sub-agent model and reasoning effort in the spawn begin event\n- render that metadata next to the spawned agent name and role in the TUI transcript --------- Co-authored-by: Codex <noreply@openai.com>
Ahmed Ibrahim ·
2026-03-11 12:33:09 -07:00 -
unifying all image saves to /tmp to bug-proof (#14149)
image-gen feature will have the model saving to /tmp by default + at all times
Won Park ·
2026-03-11 12:33:08 -07:00 -
[apps] Fix apps enablement condition. (#14011)
- [x] Fix apps enablement condition to check both the feature flag and that the user is not an API key user.
Matthew Zeng ·
2026-03-09 22:25:43 -07:00