Repository navigation
Merge upstream/main - #136
Merged
Merged
Conversation
## Summary Adds a unicode scenario, and fills in files on failing scenarios to ensure directory state is unchanged, for completeness ## Testing - [x] only changes tests
1. Adds SkillScope::Public end-to-end (core + protocol) and loads skills from the public cache directory 2. Improves repo skill discovery by searching upward for the nearest .codex/skills within a git repo 3. Deduplicates skills by name with deterministic ordering to avoid duplicates across sources 4. Fixes garbled “Skill errors” overlay rendering by preventing pending history lines from being injected during the modal 5. Updates the project docs “Skills” intro wording to avoid hardcoded paths
…le enum (#8095) This attempts to tighten up the types related to "config layers." Currently, `ConfigLayerEntry` is defined as follows: https://github.com/openai/codex/blob/bef36f4ae765f471d7cd69372fcf1b92c8f0367a/codex-rs/core/src/config_loader/state.rs#L19-L25 but the `source` field is a bit of a lie, as: - for `ConfigLayerName::Mdm`, it is `"com.openai.codex/config_toml_base64"` - for `ConfigLayerName::SessionFlags`, it is `"--config"` - for `ConfigLayerName::User`, it is `"config.toml"` (just the file name, not the path to the `config.toml` on disk that was read) - for `ConfigLayerName::System`, it seems like it is usually `/etc/codex/managed_config.toml` in practice, though on Windows, it is `%CODEX_HOME%/managed_config.toml`: https://github.com/openai/codex/blob/bef36f4ae765f471d7cd69372fcf1b92c8f0367a/codex-rs/core/src/config_loader/layer_io.rs#L84-L101 All that is to say, in three out of the four `ConfigLayerName`, `source` is a `PathBuf` that is not an absolute path (or even a true path). This PR tries to uplevel things by eliminating `source` from `ConfigLayerEntry` and turning `ConfigLayerName` into a disjoint union named `ConfigLayerSource` that has the appropriate metadata for each variant, favoring the use of `AbsolutePathBuf` where appropriate: ```rust pub enum ConfigLayerSource { /// Managed preferences layer delivered by MDM (macOS only). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] Mdm { domain: String, key: String }, /// Managed config layer from a file (usually `managed_config.toml`). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] System { file: AbsolutePathBuf }, /// Session-layer overrides supplied via `-c`/`--config`. SessionFlags, /// User config layer from a file (usually `config.toml`). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] User { file: AbsolutePathBuf }, } ```
Constrain `approval_policy` through new `admin_policy` config. This PR will: 1. Add a `admin_policy` section to config, with a single field (for now) `allowed_approval_policies`. This list constrains the set of user-settable `approval_policy`s. 2. Introduce a new `Constrained<T>` type, which combines a current value and a validator function. The validator function ensures disallowed values are not set. 3. Change the type of `approval_policy` on `Config` and `SessionConfiguration` from `AskForApproval` to `Constrained<AskForApproval>`. The validator function is set by the values passed into `allowed_approval_policies`. 4. `GenericDisplayRow`: add a `disabled_reason: Option<String>`. When set, it disables selection of the value and indicates as such in the menu. This also makes it unselectable with arrow keys or numbers. This is used in the `/approvals` menu. Follow ups are: 1. Do the same thing to `sandbox_policy`. 2. Propagate the allowed set of values through app-server for the extension (though already this should prevent app-server from setting this values, it's just that we want to disable UI elements that are unsettable). Happy to split this PR up if you prefer, into the logical numbered areas above. Especially if there are parts we want to gavel on separately (e.g. admin_policy). Disabled full access: <img width="1680" height="380" alt="image" src="https://github.com/user-attachments/assets/1fb61c8c-1fcb-4dc4-8355-2293edb52ba0" /> Disabled `--yolo` on startup: <img width="749" height="76" alt="image" src="https://github.com/user-attachments/assets/0a1211a0-6eb1-40d6-a1d7-439c41e94ddb" /> CODEX-4087
This will automatically render any `Stage::Beta` features. The change only gets applied to the *next session*. This started as a bug but actually this is a good thing to prevent out of distribution push <img width="986" height="288" alt="Screenshot 2025-12-15 at 15 38 35" src="https://github.com/user-attachments/assets/78b7a71d-0e43-4828-a118-91c5237909c7" /> <img width="509" height="109" alt="Screenshot 2025-12-15 at 17 35 44" src="https://github.com/user-attachments/assets/6933de52-9b66-4abf-b58b-a5f26d5747e2" />
# With `unified_exec` Known tools are correctly casted <img width="1150" height="312" alt="Screenshot 2025-12-16 at 19 27 28" src="https://github.com/user-attachments/assets/24150ee5-e88d-461b-a459-483c24784196" /> If a session exit the turn, we render it with the "Ran ..." <img width="1168" height="355" alt="Screenshot 2025-12-16 at 19 27 58" src="https://github.com/user-attachments/assets/3f00b60c-2d57-4f9d-a201-9cc8388957cb" /> If a session does not exit during the turn, it is closed at the end of the turn but this is not rendered <img width="642" height="342" alt="Screenshot 2025-12-16 at 19 34 37" src="https://github.com/user-attachments/assets/c2bd9283-7017-4915-ba73-c52199b0b28e" /> # Without `unified_exec` No changes <img width="740" height="603" alt="Screenshot 2025-12-16 at 19 31 21" src="https://github.com/user-attachments/assets/ca5d90fe-a9b2-42ba-bcd7-3e98c4ed22e8" />
### Summary * Update `listMcpServerStatus` to be non-blocking by wrapping it with tokio:spawn.
The problem is that the `tokio` task own an `Arc` reference of the session and that this task only exit with the broadcast channel get closed. But this never get closed if the session is not dropped. So it's a snake biting his tail basically The most notable result was that non of the `Drop` implementation were triggered (temporary files, shell snapshots, session cleaning etc etc) when closing the session (through a `/new` for example) The fix is just to weaken the `Arc` and upgrade it on the fly
We should not have any `PathBuf` fields in `ConfigToml` or any of the transitive structs we include, as we should use `AbsolutePathBuf` instead so that we do not have to keep track of the file from which `ConfigToml` was loaded such that we need it to resolve relative paths later when the values of `ConfigToml` are used. I only found two instances of this: `experimental_instructions_file` and `experimental_compact_prompt_file`. Incidentally, when these were specified as relative paths, they were resolved against `cwd` rather than `config.toml`'s parent, which seems wrong to me. I changed the behavior so they are resolved against the parent folder of the `config.toml` being parsed, which we get "for free" due to the introduction of `AbsolutePathBufGuard ` in openai/codex#7796. While it is not great to change the behavior of a released feature, these fields are prefixed with `experimental_`, which I interpret to mean we have the liberty to change the contract. For reference: - `experimental_instructions_file` was introduced in openai/codex#1803 - `experimental_compact_prompt_file` was introduced in openai/codex#5959
### Summary * Make `app_server.list_models` to be non-blocking and consumers (i.e. extension) can manage the flow themselves. * Force config to use remote models and therefore fetch codex-auto model list.
- Load models from static file as a fallback - Make API users use this file directly - Add tests to make sure updates to the file always serialize
- Batch read ACL creation for online/offline sandbox user - creates a new ACL helper process that is long-lived and runs in the background - uses a mutex so that only one helper process is running at a time.
# External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request.
This PR does various types of cleanup before I can proceed with more ambitious changes to config loading. First, I noticed duplicated code across these two methods: https://github.com/openai/codex/blob/774bd9e432fa2e0f4e059e97648cf92216912e19/codex-rs/core/src/config/mod.rs#L314-L324 https://github.com/openai/codex/blob/774bd9e432fa2e0f4e059e97648cf92216912e19/codex-rs/core/src/config/mod.rs#L334-L344 This has now been consolidated in `load_config_as_toml_with_cli_overrides()`. Further, I noticed that `Config::load_with_cli_overrides()` took two similar arguments: https://github.com/openai/codex/blob/774bd9e432fa2e0f4e059e97648cf92216912e19/codex-rs/core/src/config/mod.rs#L308-L311 The difference between `cli_overrides` and `overrides` was not immediately obvious to me. At first glance, it appears that one should be able to be expressed in terms of the other, but it turns out that some fields of `ConfigOverrides` (such as `cwd` and `codex_linux_sandbox_exe`) are, by design, not configurable via a `.toml` file or a command-line `--config` flag. That said, I discovered that many callers of `Config::load_with_cli_overrides()` were passing `ConfigOverrides::default()` for `overrides`, so I created two separate methods: - `Config::load_with_cli_overrides(cli_overrides: Vec<(String, TomlValue)>)` - `Config::load_with_cli_overrides_and_harness_overrides(cli_overrides: Vec<(String, TomlValue)>, harness_overrides: ConfigOverrides)` The latter has a long name, as it is _not_ what should be used in the common case, so the extra typing is designed to draw attention to this fact. I tried to update the existing callsites to use the shorter name, where possible. Further, in the cases where `ConfigOverrides` is used, usually only a limited subset of fields are actually set, so I updated the declarations to leverage `..Default::default()` where possible.
# External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request.
1. Remove PUBLIC skills and introduce SYSTEM skills embedded in the binary and installed into $CODEX_HOME/skills/.system at startup. 2. Skills are now always enabled (feature flag removed). 3. Update skills/list to accept forceReload and plumb it through (not used by clients yet).
Instead of failing to start Codex, clearly call out that N skills did not load and provide warnings so that the user may fix them. <img width="3548" height="874" alt="image" src="https://github.com/user-attachments/assets/6ce041b2-1373-4007-a6dd-0194e58fafe4" />
Introduce `ConfigBuilder` as an alternative to our existing `Config` constructors. I noticed that the existing constructors, `Config::load_with_cli_overrides()` and `Config::load_with_cli_overrides_and_harness_overrides()`, did not take `codex_home` as a parameter, which can be a problem. Historically, when Codex was purely a CLI, we wanted to be extra sure that the creation of `codex_home` was always done via `find_codex_home()`, so we did not expose `codex_home` as a parameter when creating `Config` in business logic. But in integration tests, `codex_home` nearly always needs to be configured (as a temp directory), which is why callers would have to go through `Config::load_from_base_config_with_overrides()` instead. Now that the Codex harness also functions as an app server, which could conceivably load multiple threads where `codex_home` is parameterized differently in each one, I think it makes sense to make this configurable. Going to a builder pattern makes it more flexible to ensure an arbitrary permutation of options can be set when constructing a `Config` while using the appropriate defaults for the options that aren't set explicitly. Ultimately, I think this should make it possible for us to make `Config::load_from_base_config_with_overrides()` private because all integration tests should be able to leverage `ConfigBuilder` instead. Though there could be edge cases, so I'll pursue that migration after we get through the current config overhaul. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8235). * #8237 * __->__ #8235
This pull request updates the ChatGPT login description in the onboarding authentication widgets to clarify which plans include usage. The description now lists "Business" rather than "Team" and adds "Education" plans in addition to the previously mentioned plans. I have read the CLA Document and I hereby sign the CLAs. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1. Reintroduce feature flags for skills; 2. UI tweaks (truncate descriptions, better validation error display).
Over in `config_loader/macos.rs`, we were doing this complicated `mod` thing to expose one version of `load_managed_admin_config_layer()` for Mac: https://github.com/openai/codex/blob/580c59aa9af61cb4bffb5b204bd16a5dcc4bc911/codex-rs/core/src/config_loader/macos.rs#L4-L5 While exposing a trivial implementation for non-Mac: https://github.com/openai/codex/blob/580c59aa9af61cb4bffb5b204bd16a5dcc4bc911/codex-rs/core/src/config_loader/macos.rs#L110-L117 That was being used like this: https://github.com/openai/codex/blob/580c59aa9af61cb4bffb5b204bd16a5dcc4bc911/codex-rs/core/src/config_loader/layer_io.rs#L47-L48 This PR simplifies that callsite in `layer_io.rs` to just be: ```rust #[cfg(not(target_os = "macos"))] let managed_preferences = None; ``` And updates `config_loader/mod.rs` so we only pull in `macos.rs` on Mac: ```rust #[cfg(target_os = "macos")] mod macos; ``` This simplifies `macos.rs` considerably, though it looks like a big change because everything gets unindented and reformatted because we can drop the whole `mod native` thing now. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8248). * #8251 * #8249 * __->__ #8248
# External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request.
This is some minor API cleanup that will make it easier to use `AbsolutePathBuf` in more places in a subsequent PR.
This pull request makes a small update to the session picker documentation for `codex resume`. The main change clarifies how to view the original working directory (CWD) for sessions and when the Git branch is shown. - The session picker now displays the recorded Git branch when available, and instructions are added for showing the original working directory by using the `--all` flag, which also disables CWD filtering and adds a `CWD` column.
Welcome caribou <img width="1536" height="1024" alt="image" src="https://github.com/user-attachments/assets/2a67b21f-40cf-4518-aee4-691af331ab50" />
Add a name to Beta features <img width="906" height="153" alt="Screenshot 2025-12-18 at 16 42 49" src="https://github.com/user-attachments/assets/d56f3519-0613-4d9a-ad4d-38b1a7eb125a" />
## Summary - add a shared git-ref resolver and use it for `codex cloud exec` and TUI task submission - expose a new `--branch` flag to override the git ref passed to cloud tasks - cover the git-ref resolution behavior with new async unit tests and supporting dev dependencies ## Testing - cargo test -p codex-cloud-tasks ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_692decc6cbec8332953470ef063e11ab) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Co-authored-by: Jeremy Rose <nornagon@openai.com>
This is a significant change to how layers of configuration are applied. In particular, the `ConfigLayerStack` now has two important fields: - `layers: Vec<ConfigLayerEntry>` - `requirements: ConfigRequirements` We merge `TomlValue`s across the layers, but they are subject to `ConfigRequirements` before creating a `Config`. How I would review this PR: - start with `codex-rs/app-server-protocol/src/protocol/v2.rs` and note the new variants added to the `ConfigLayerSource` enum: `LegacyManagedConfigTomlFromFile` and `LegacyManagedConfigTomlFromMdm` - note that `ConfigLayerSource` now has a `precedence()` method and implements `PartialOrd` - `codex-rs/core/src/config_loader/layer_io.rs` is responsible for loading "admin" preferences from `/etc/codex/managed_config.toml` and MDM. Because `/etc/codex/managed_config.toml` is now deprecated in favor of `/etc/codex/requirements.toml` and `/etc/codex/config.toml`, we now include some extra information on the `LoadedConfigLayers` returned in `layer_io.rs`. - `codex-rs/core/src/config_loader/mod.rs` has major changes to `load_config_layers_state()`, which is what produces `ConfigLayerStack`. The docstring has the new specification and describes the various layers that will be loaded and the precedence order. - It uses the information from `LoaderOverrides` "twice," both in the spirit of legacy support: - We use one instances to derive an instance of `ConfigRequirements`. Currently, the only field in `managed_config.toml` that contributes to `ConfigRequirements` is `approval_policy`. This PR introduces `Constrained::allow_only()` to support this. - We use a clone of `LoaderOverrides` to derive `ConfigLayerSource::LegacyManagedConfigTomlFromFile` and `ConfigLayerSource::LegacyManagedConfigTomlFromMdm` layers, as appropriate. As before, this ends up being a "best effort" at enterprise controls, but is enforcement is not guaranteed like it is for `ConfigRequirements`. - Now we only create a "user" layer if `$CODEX_HOME/config.toml` exists. (Previously, a user layer was always created for `ConfigLayerStack`.) - Similarly, we only add a "session flags" layer if there are CLI overrides. - `config_loader/state.rs` contains the updated implementation for `ConfigLayerStack`. Note the public API is largely the same as before, but the implementation is quite different. We leverage the fact that `ConfigLayerSource` is now `PartialOrd` to ensure layers are in the correct order. - A `Config` constructed via `ConfigBuilder.build()` will use `load_config_layers_state()` to create the `ConfigLayerStack` and use the associated `ConfigRequirements` when constructing the `Config` object. - That said, a `Config` constructed via `Config::load_from_base_config_with_overrides()` does _not_ yet use `ConfigBuilder`, so it creates a `ConfigRequirements::default()` instead of loading a proper `ConfigRequirements`. I will fix this in a subsequent PR. Then the following files are mostly test changes: ``` codex-rs/app-server/tests/suite/v2/config_rpc.rs codex-rs/core/src/config/service.rs codex-rs/core/src/config_loader/tests.rs ``` Again, because we do not always include "user" and "session flags" layers when the contents are empty, `ConfigLayerStack` sometimes has fewer layers than before (and the precedence order changed slightly), which is the main reason integration tests changed.
# External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request.
before: <img width="795" height="150" alt="Screenshot 2025-12-18 at 10 48 01 AM" src="https://github.com/user-attachments/assets/6f4d8856-b4c2-4e2a-b60a-b86f82b956a0" /> after: <img width="795" height="150" alt="Screenshot 2025-12-18 at 10 48 39 AM" src="https://github.com/user-attachments/assets/dd0d167a-5d09-4bb7-9d36-95a2eb1aaa83" />
Add a dmg target that bundles the codex and codex responses api proxy binaries for MacOS. this target is signed and notarized. Verified by triggering a build here: https://github.com/openai/codex/actions/runs/20318136302/job/58367155205. Downloaded the artifact and verified that the dmg is signed and notarized, and the codex binary contained works as expected.
…er (#8275) when granting read access to the sandbox user, grant the codex/command-runner exe directory first so commands can run before the entire read ACL process is finished.
|
The codespell checker was incorrectly flagging 'iterm' in terminal.rs as a typo when it's actually the correct terminal name. Added to ignore list. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync fork with upstream main (merged upstream/main into update123).