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>
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
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).
# 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-L324https://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.
- 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.
### 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.
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
https://github.com/openai/codex/pull/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
https://github.com/openai/codex/pull/1803
- `experimental_compact_prompt_file` was introduced in
https://github.com/openai/codex/pull/5959
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
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 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 },
}
```
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
## 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
Update the tui2 viewport/history design doc with current status and a
prioritized roadmap (scroll feel, selection/copy correctness, streaming
wrap polish, terminal integration, and longer-term per-cell
interactivity ideas).
## Summary:
This PR is a pure copy and paste of tests from lib.rs into
invocation.rs, to colocate logic and tests.
## Testing
- [x] Purely a test refactor
This reverts commit 291b54a762.
This commit was intended to prevent the model from making code changes
during `/review`, which is sometimes does. Unfortunately, it has other
unintended side effects that cause `/review` to fail in a variety of
ways. See #8115 and #7815. We've therefore decided to revert this
change.
I'm not sure if this fix is correct for the intended change in #7601,
but at least the compilation error is fixed.
regression: #7601
```
error[E0004]: non-exhaustive patterns: `TuiEvent::Mouse(_)` not covered
--> tui2/src/update_prompt.rs:57:19
|
57 | match event {
| ^^^^^ pattern `TuiEvent::Mouse(_)` not covered
|
note: `TuiEvent` defined here
--> tui2/src/tui.rs:122:10
|
122 | pub enum TuiEvent {
| ^^^^^^^^
...
126 | Mouse(crossterm::event::MouseEvent),
| ----- not covered
= note: the matched value is of type `TuiEvent`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
64 ~ },
65 + TuiEvent::Mouse(_) => todo!()
|
```
Signed-off-by: Koichi Shiraishi <zchee.io@gmail.com>
lib.rs has grown quite large, and mixes two responsibilities:
1. executing patch operations
2. parsing apply_patch invocations via a shell command
This PR splits out (2) into its own file, so we can work with it more
easily. We are explicitly NOT moving tests in this PR, to ensure
behavior stays the same and we can avoid losing coverage via merge
conflicts. Tests are moved in a subsequent PR.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8110).
* #8111
* __->__ #8110
a few fixes based on testing feedback:
* ensure cap_sid file is always written by elevated setup.
* always log to same file whether using elevated sandbox or not
* process potentially slow ACE write operations in parallel
* dedupe write roots so we don't double process any
* don't try to create read/write ACEs on the same directories, due to
race condition
Introduces an `EventBroker` between the crossterm `EventStream` source
and the consumers in the TUI. This enables dropping + recreating the
`crossterm_events` without invalidating the consumer.
Dropping and recreating the crossterm event stream enables us to fully
relinquish `stdin` while the app keeps running. If the stream is not
dropped, it will continue to read from `stdin` even when it is not
actively being polled, potentially stealing input from other processes.
See
[here](https://www.reddit.com/r/rust/comments/1f3o33u/myterious_crossterm_input_after_running_vim/?utm_source=chatgpt.com)
and [here](https://ratatui.rs/recipes/apps/spawn-vim/) for details.
### Tests
Added tests for new `EventBroker` setup, existing tests pass, tested
locally.
In preparation for in-repo configuration support, this updates
`WritableRoot::get_writable_roots_with_cwd()` to include the `.codex`
subfolder in `WritableRoot.read_only_subpaths`, if it exists, as we
already do for `.git`.
As noted, currently, like `.git`, `.codex` will only be read-only under
macOS Seatbelt, but we plan to bring support to other OSes, as well.
Updated the integration test in `seatbelt.rs` so that it actually
attempts to run the generated Seatbelt commands, verifying that:
- trying to write to `.codex/config.toml` in a writable root fails
- trying to write to `.git/hooks/pre-commit` in a writable root fails
- trying to write to the writable root containing the `.codex` and
`.git` subfolders succeeds
This is a pure refactor only change.
Replace the flattened transcript line metadata from `Option<(usize,
usize)>` to an explicit
`TranscriptLineMeta::{CellLine { cell_index, line_in_cell }, Spacer}`
enum.
This makes spacer rows unambiguous, removes “tuple semantics” from call
sites, and keeps the
scroll anchoring model clearer and aligned with the viewport/history
design notes.
Changes:
- Introduce `TranscriptLineMeta` and update `TranscriptScroll` helpers
to consume it.
- Update `App::build_transcript_lines` and downstream consumers
(scrolling, row classification, ANSI rendering).
- Refresh scrolling module docs to describe anchors + spacer semantics
in context.
- Add tests and docs about the behavior
Tests:
- just fmt
- cargo test -p codex-tui2 tui::scrolling
Manual testing:
- Scroll the inline transcript with mouse wheel + PgUp/PgDn/Home/End,
then resize the terminal while staying scrolled up; verify the same
anchored content stays in view and you don’t jump to bottom
unexpectedly.
- Create a gap case (multiple non-continuation cells) and scroll so a
blank spacer row is at/near the top; verify scrolling doesn’t get stuck
on spacers and still anchors to nearby real lines.
- Start a selection while the assistant is streaming; verify the view
stops auto-following, the selection stays on the intended content, and
subsequent scrolling still behaves normally.
- Exit the TUI and confirm scrollback rendering still styles user rows
as blocks (background padding) and non-user rows as expected.
Michael BolinandGitHub
jif-oaiandGitHub
xl-openaiandGitHub
Gav VermaandGitHub
Ahmed IbrahimandGitHub
iceweasel-oaiandGitHub
Shijie RaoandGitHub
gt-oaiandGitHub
Dylan HurdandGitHub
Celia ChenandGitHub
Eric TrautandGitHub
Josh McKinneyandGitHub
Koichi ShiraishiandGitHub
sayan-oaiandGitHub