Certain shell commands are potentially dangerous, and we want to check
for them.
Unless the user has explicitly approved a command, we will *always* ask
them for approval
when one of these commands is encountered, regardless of whether they
are in a sandbox, or what their approval policy is.
The first (of probably many) such examples is `git reset --hard`. We
will be conservative and check for any `git reset`
This addresses bug #4092
Testing:
* Confirmed error occurs prior to fix if logging in using API key and no
`~/.codex` directory exists
* Confirmed after fix that `~/.codex` directory is properly created and
error doesn't occur
# 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.
This changes the reqwest client used in tests to be sandbox-friendly,
and skips a bunch of other tests that don't work inside the
sandbox/without network.
I would like to be able to swap in a different way to resolve model
sampling requests, so this refactoring consolidates things behind
`attempt_stream_responses()` to make that easier. Ideally, we would
support an in-memory backend that we can use in our integration tests,
for example.
## Current State Observations
- `Session` currently holds many unrelated responsibilities (history,
approval queues, task handles, rollout recorder, shell discovery, token
tracking, etc.), making it hard to reason about ownership and lifetimes.
- The anonymous `State` struct inside `codex.rs` mixes session-long data
with turn-scoped queues and approval bookkeeping.
- Turn execution (`run_task`) relies on ad-hoc local variables that
should conceptually belong to a per-turn state object.
- External modules (`codex::compact`, tests) frequently poke the raw
`Session.state` mutex, which couples them to implementation details.
- Interrupts, approvals, and rollout persistence all have bespoke
cleanup paths, contributing to subtle bugs when a turn is aborted
mid-flight.
## Desired End State
- Keep a slim `Session` object that acts as the orchestrator and façade.
It should expose a focused API (submit, approvals, interrupts, event
emission) without storing unrelated fields directly.
- Introduce a `state` module that encapsulates all mutable data
structures:
- `SessionState`: session-persistent data (history, approved commands,
token/rate-limit info, maybe user preferences).
- `ActiveTurn`: metadata for the currently running turn (sub-id, task
kind, abort handle) and an `Arc<TurnState>`.
- `TurnState`: all turn-scoped pieces (pending inputs, approval waiters,
diff tracker, review history, auto-compact flags, last agent message,
outstanding tool call bookkeeping).
- Group long-lived helpers/managers into a dedicated `SessionServices`
struct so `Session` does not accumulate "random" fields.
- Provide clear, lock-safe APIs so other modules never touch raw
mutexes.
- Ensure every turn creates/drops a `TurnState` and that
interrupts/finishes delegate cleanup to it.
For the most part, we try to avoid environment variables in favor of
config options so the environment variables do not leak into child
processes. These environment variables are no longer honored, so let's
delete them to be clear.
Ultimately, I would also like to eliminate `CODEX_RS_SSE_FIXTURE` in
favor of something cleaner.
refactors command_safety files into its own package, so we can add
platform-specific ones
Also creates a windows-specific of `is_known_safe_command` that just
returns false always, since that is what happens today.
## Summary
- refactor the stream retry integration tests to construct conversations
through `TestCodex`
- remove bespoke config and tempdir setup now handled by the shared
builder
## Testing
- cargo test -p codex-core --test all
stream_error_allows_next_turn::continue_after_stream_error
- cargo test -p codex-core --test all
stream_no_completed::retries_on_early_close
------
https://chatgpt.com/codex/tasks/task_i_68d2b94d83888320bc75a0bc3bd77b49
The only file to watch is the cargo.toml
All the others come from just fix + a few manual small fix
The set of rules have been taken from the list of clippy rules
arbitrarily while trying to optimise the learning and style of the code
while limiting the loss of productivity
Adds the following options:
1. Review current changes
2. Review a specific commit
3. Review against a base branch (PR style)
4. Custom instructions
<img width="487" height="330" alt="Screenshot 2025-09-20 at 2 11 36 PM"
src="https://github.com/user-attachments/assets/edb0aaa5-5747-47fa-881f-cc4c4f7fe8bc"
/>
---
\+ Adds the following UI helpers:
1. Makes list selection searchable
2. Adds navigation to the bottom pane, so you could add a stack of
popups
3. Basic custom prompt view
We currently get information about rate limits in the response headers.
We want to forward them to the clients to have better transparency.
UI/UX plans have been discussed and this information is needed.
Currently, we change the tool description according to the sandbox
policy and approval policy. This breaks the cache when the user hits
`/approvals`. This PR does the following:
- Always use the shell with escalation parameter:
- removes `create_shell_tool_for_sandbox` and always uses unified tool
via `create_shell_tool`
- Reject the func call when the model uses escalation parameter when it
cannot.
### Why Use `tokio::sync::Mutex`
`std::sync::Mutex` are not _async-aware_. As a result, they will block
the entire thread instead of just yielding the task. Furthermore they
can be poisoned which is not the case of `tokio` Mutex.
This allows the Tokio runtime to continue running other tasks while
waiting for the lock, preventing deadlocks and performance bottlenecks.
In general, this is preferred in async environment
Proposal: We want to record a dev message like so:
```
{
"type": "message",
"role": "user",
"content": [
{
"type": "input_text",
"text": "<user_action>
<context>User initiated a review task. Here's the full review output from reviewer model. User may select one or more comments to resolve.</context>
<action>review</action>
<results>
{findings_str}
</results>
</user_action>"
}
]
},
```
Without showing in the chat transcript.
Rough idea, but it fixes issue where the user finishes a review thread,
and asks the parent "fix the rest of the review issues" thinking that
the parent knows about it.
### Question: Why not a tool call?
Because the agent didn't make the call, it was a human. + we haven't
implemented sub-agents yet, and we'll need to think about the way we
represent these human-led tool calls for the agent.
