mirror of
https://github.com/openai/codex.git
synced 2026-04-23 07:51:51 +03:00
Compare commits
8 Commits
nornagon/w
...
dh--cache-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e332154dd4 | ||
|
|
3e8bcf0247 | ||
|
|
cda39e417f | ||
|
|
d642b07fcc | ||
|
|
7b3ab968a0 | ||
|
|
02e7965228 | ||
|
|
493e4c9463 | ||
|
|
1f7003b476 |
128
.github/codex/labels/codex-rust-review.md
vendored
128
.github/codex/labels/codex-rust-review.md
vendored
@@ -6,18 +6,134 @@ Then provide the **review** (1-2 sentences plus bullet points, friendly tone).
|
||||
|
||||
Things to look out for when doing the review:
|
||||
|
||||
## General Principles
|
||||
|
||||
- **Make sure the pull request body explains the motivation behind the change.** If the author has failed to do this, call it out, and if you think you can deduce the motivation behind the change, propose copy.
|
||||
- Ideally, the PR body also contains a small summary of the change. For small changes, the PR title may be sufficient.
|
||||
- Each PR should ideally do one conceptual thing. For example, if a PR does a refactoring as well as introducing a new feature, push back and suggest the refactoring be done in a separate PR. This makes things easier for the reviewer, as refactoring changes can often be far-reaching, yet quick to review.
|
||||
- If the nature of the change seems to have a visual component (which is often the case for changes to `codex-rs/tui`), recommend including a screenshot or video to demonstrate the change, if appropriate.
|
||||
- Rust files should generally be organized such that the public parts of the API appear near the top of the file and helper functions go below. This is analagous to the "inverted pyramid" structure that is favored in journalism.
|
||||
- Encourage the use of small enums or the newtype pattern in Rust if it helps readability without adding significant cognitive load or lines of code.
|
||||
- Be wary of large files and offer suggestions for how to break things into more reasonably-sized files.
|
||||
- When modifying a `Cargo.toml` file, make sure that dependency lists stay alphabetically sorted. Also consider whether a new dependency is added to the appropriate place (e.g., `[dependencies]` versus `[dev-dependencies]`)
|
||||
- If you see opportunities for the changes in a diff to use more idiomatic Rust, please make specific recommendations. For example, favor the use of expressions over `return`.
|
||||
- When introducing new code, be on the lookout for code that duplicates existing code. When found, propose a way to refactor the existing code such that it should be reused.
|
||||
|
||||
## Code Organization
|
||||
|
||||
- Each create in the Cargo workspace in `codex-rs` has a specific purpose: make a note if you believe new code is not introduced in the correct crate.
|
||||
- When possible, try to keep the `core` crate as small as possible. Non-core but shared logic is often a good candidate for `codex-rs/common`.
|
||||
- Be wary of large files and offer suggestions for how to break things into more reasonably-sized files.
|
||||
- Rust files should generally be organized such that the public parts of the API appear near the top of the file and helper functions go below. This is analagous to the "inverted pyramid" structure that is favored in journalism.
|
||||
|
||||
## Assertions in Tests
|
||||
|
||||
Assert the equality of the entire objects instead of doing "piecemeal comparisons," performing `assert_eq!()` on individual fields.
|
||||
|
||||
Note that unit tests also function as "executable documentation." As shown in the following example, "piecemeal comparisons" are often more verbose, provide less coverage, and are not as useful as executable documentation.
|
||||
|
||||
For example, suppose you have the following enum:
|
||||
|
||||
```rust
|
||||
#[derive(Debug, PartialEq)]
|
||||
enum Message {
|
||||
Request {
|
||||
id: String,
|
||||
method: String,
|
||||
params: Option<serde_json::Value>,
|
||||
},
|
||||
Notification {
|
||||
method: String,
|
||||
params: Option<serde_json::Value>,
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
This is an example of a _piecemeal_ comparison:
|
||||
|
||||
```rust
|
||||
// BAD: Piecemeal Comparison
|
||||
|
||||
#[test]
|
||||
fn test_get_latest_messages() {
|
||||
let messages = get_latest_messages();
|
||||
assert_eq!(messages.len(), 2);
|
||||
|
||||
let m0 = &messages[0];
|
||||
match m0 {
|
||||
Message::Request { id, method, params } => {
|
||||
assert_eq!(id, "123");
|
||||
assert_eq!(method, "subscribe");
|
||||
assert_eq!(
|
||||
*params,
|
||||
Some(json!({
|
||||
"conversation_id": "x42z86"
|
||||
}))
|
||||
)
|
||||
}
|
||||
Message::Notification { .. } => {
|
||||
panic!("expected Request");
|
||||
}
|
||||
}
|
||||
|
||||
let m1 = &messages[1];
|
||||
match m1 {
|
||||
Message::Request { .. } => {
|
||||
panic!("expected Notification");
|
||||
}
|
||||
Message::Notification { method, params } => {
|
||||
assert_eq!(method, "log");
|
||||
assert_eq!(
|
||||
*params,
|
||||
Some(json!({
|
||||
"level": "info",
|
||||
"message": "subscribed"
|
||||
}))
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This is a _deep_ comparison:
|
||||
|
||||
```rust
|
||||
// GOOD: Verify the entire structure with a single assert_eq!().
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn test_get_latest_messages() {
|
||||
let messages = get_latest_messages();
|
||||
|
||||
assert_eq!(
|
||||
vec![
|
||||
Message::Request {
|
||||
id: "123".to_string(),
|
||||
method: "subscribe".to_string(),
|
||||
params: Some(json!({
|
||||
"conversation_id": "x42z86"
|
||||
})),
|
||||
},
|
||||
Message::Notification {
|
||||
method: "log".to_string(),
|
||||
params: Some(json!({
|
||||
"level": "info",
|
||||
"message": "subscribed"
|
||||
})),
|
||||
},
|
||||
],
|
||||
messages,
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
## More Tactical Rust Things To Look Out For
|
||||
|
||||
- Do not use `unsafe` (unless you have a really, really good reason like using an operating system API directly and no safe wrapper exists). For example, there are cases where it is tempting to use `unsafe` in order to use `std::env::set_var()`, but this indeed `unsafe` and has led to race conditions on multiple occasions. (When this happens, find a mechanism other than environment variables to use for configuration.)
|
||||
- Encourage the use of small enums or the newtype pattern in Rust if it helps readability without adding significant cognitive load or lines of code.
|
||||
- If you see opportunities for the changes in a diff to use more idiomatic Rust, please make specific recommendations. For example, favor the use of expressions over `return`.
|
||||
- When modifying a `Cargo.toml` file, make sure that dependency lists stay alphabetically sorted. Also consider whether a new dependency is added to the appropriate place (e.g., `[dependencies]` versus `[dev-dependencies]`)
|
||||
|
||||
## Pull Request Body
|
||||
|
||||
- If the nature of the change seems to have a visual component (which is often the case for changes to `codex-rs/tui`), recommend including a screenshot or video to demonstrate the change, if appropriate.
|
||||
- References to existing GitHub issues and PRs are encouraged, where appropriate, though you likely do not have network access, so may not be able to help here.
|
||||
|
||||
# PR Information
|
||||
|
||||
{CODEX_ACTION_GITHUB_EVENT_PATH} contains the JSON that triggered this GitHub workflow. It contains the `base` and `head` refs that define this PR. Both refs are available locally.
|
||||
|
||||
6
.github/workflows/rust-release.yml
vendored
6
.github/workflows/rust-release.yml
vendored
@@ -181,9 +181,9 @@ jobs:
|
||||
name: ${{ steps.release_name.outputs.name }}
|
||||
tag_name: ${{ github.ref_name }}
|
||||
files: dist/**
|
||||
# For now, tag releases as "prerelease" because we are not claiming
|
||||
# the Rust CLI is stable yet.
|
||||
prerelease: true
|
||||
# Mark as prerelease only when the version has a suffix after x.y.z
|
||||
# (e.g. -alpha, -beta). Otherwise publish a normal release.
|
||||
prerelease: ${{ contains(steps.release_name.outputs.name, '-') }}
|
||||
|
||||
- uses: facebook/dotslash-publish-release@v2
|
||||
env:
|
||||
|
||||
29
codex-rs/common/src/config_summary.rs
Normal file
29
codex-rs/common/src/config_summary.rs
Normal file
@@ -0,0 +1,29 @@
|
||||
use codex_core::WireApi;
|
||||
use codex_core::config::Config;
|
||||
|
||||
use crate::sandbox_summary::summarize_sandbox_policy;
|
||||
|
||||
/// Build a list of key/value pairs summarizing the effective configuration.
|
||||
pub fn create_config_summary_entries(config: &Config) -> Vec<(&'static str, String)> {
|
||||
let mut entries = vec![
|
||||
("workdir", config.cwd.display().to_string()),
|
||||
("model", config.model.clone()),
|
||||
("provider", config.model_provider_id.clone()),
|
||||
("approval", config.approval_policy.to_string()),
|
||||
("sandbox", summarize_sandbox_policy(&config.sandbox_policy)),
|
||||
];
|
||||
if config.model_provider.wire_api == WireApi::Responses
|
||||
&& config.model_family.supports_reasoning_summaries
|
||||
{
|
||||
entries.push((
|
||||
"reasoning effort",
|
||||
config.model_reasoning_effort.to_string(),
|
||||
));
|
||||
entries.push((
|
||||
"reasoning summaries",
|
||||
config.model_reasoning_summary.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
entries
|
||||
}
|
||||
@@ -23,3 +23,7 @@ mod sandbox_summary;
|
||||
|
||||
#[cfg(feature = "sandbox_summary")]
|
||||
pub use sandbox_summary::summarize_sandbox_policy;
|
||||
|
||||
mod config_summary;
|
||||
|
||||
pub use config_summary::create_config_summary_entries;
|
||||
|
||||
@@ -41,11 +41,9 @@ pub(crate) async fn stream_chat_completions(
|
||||
let full_instructions = prompt.get_full_instructions(model_family);
|
||||
messages.push(json!({"role": "system", "content": full_instructions}));
|
||||
|
||||
if let Some(instr) = &prompt.get_formatted_user_instructions() {
|
||||
messages.push(json!({"role": "user", "content": instr}));
|
||||
}
|
||||
let input = prompt.get_formatted_input();
|
||||
|
||||
for item in &prompt.input {
|
||||
for item in &input {
|
||||
match item {
|
||||
ResponseItem::Message { role, content, .. } => {
|
||||
let mut text = String::new();
|
||||
|
||||
@@ -34,7 +34,6 @@ use crate::error::Result;
|
||||
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
||||
use crate::model_provider_info::ModelProviderInfo;
|
||||
use crate::model_provider_info::WireApi;
|
||||
use crate::models::ContentItem;
|
||||
use crate::models::ResponseItem;
|
||||
use crate::openai_tools::create_tools_json_for_responses_api;
|
||||
use crate::protocol::TokenUsage;
|
||||
@@ -146,15 +145,7 @@ impl ModelClient {
|
||||
vec![]
|
||||
};
|
||||
|
||||
let mut input_with_instructions = Vec::with_capacity(prompt.input.len() + 1);
|
||||
if let Some(ui) = prompt.get_formatted_user_instructions() {
|
||||
input_with_instructions.push(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: ui }],
|
||||
});
|
||||
}
|
||||
input_with_instructions.extend(prompt.input.clone());
|
||||
let input_with_instructions = prompt.get_formatted_input();
|
||||
|
||||
let payload = ResponsesApiRequest {
|
||||
model: &self.config.model,
|
||||
|
||||
@@ -1,14 +1,20 @@
|
||||
use crate::config_types::ReasoningEffort as ReasoningEffortConfig;
|
||||
use crate::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use crate::error::Result;
|
||||
use crate::git_info::GitInfo;
|
||||
use crate::model_family::ModelFamily;
|
||||
use crate::models::ContentItem;
|
||||
use crate::models::ResponseItem;
|
||||
use crate::openai_tools::OpenAiTool;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::protocol::TokenUsage;
|
||||
use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS;
|
||||
use futures::Stream;
|
||||
use serde::Serialize;
|
||||
use std::borrow::Cow;
|
||||
use std::fmt::Display;
|
||||
use std::path::PathBuf;
|
||||
use std::pin::Pin;
|
||||
use std::task::Context;
|
||||
use std::task::Poll;
|
||||
@@ -18,10 +24,49 @@ use tokio::sync::mpsc;
|
||||
/// with this content.
|
||||
const BASE_INSTRUCTIONS: &str = include_str!("../prompt.md");
|
||||
|
||||
/// wraps environment context message in a tag for the model to parse more easily.
|
||||
const ENVIRONMENT_CONTEXT_START: &str = "<environment_context>\n\n";
|
||||
const ENVIRONMENT_CONTEXT_END: &str = "\n\n</environment_context>";
|
||||
|
||||
/// wraps user instructions message in a tag for the model to parse more easily.
|
||||
const USER_INSTRUCTIONS_START: &str = "<user_instructions>\n\n";
|
||||
const USER_INSTRUCTIONS_END: &str = "\n\n</user_instructions>";
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub(crate) struct EnvironmentContext {
|
||||
pub cwd: PathBuf,
|
||||
pub git_info: Option<GitInfo>,
|
||||
pub approval_policy: AskForApproval,
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
}
|
||||
|
||||
impl Display for EnvironmentContext {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
writeln!(
|
||||
f,
|
||||
"Current working directory: {}",
|
||||
self.cwd.to_string_lossy()
|
||||
)?;
|
||||
writeln!(f, "Is directory a git repo: {}", self.git_info.is_some())?;
|
||||
writeln!(f, "Approval policy: {}", self.approval_policy)?;
|
||||
writeln!(f, "Sandbox policy: {}", self.sandbox_policy)?;
|
||||
|
||||
let network_access = match self.sandbox_policy.clone() {
|
||||
SandboxPolicy::DangerFullAccess => "enabled",
|
||||
SandboxPolicy::ReadOnly => "restricted",
|
||||
SandboxPolicy::WorkspaceWrite { network_access, .. } => {
|
||||
if network_access {
|
||||
"enabled"
|
||||
} else {
|
||||
"restricted"
|
||||
}
|
||||
}
|
||||
};
|
||||
writeln!(f, "Network access: {network_access}")?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// API request payload for a single model turn.
|
||||
#[derive(Default, Debug, Clone)]
|
||||
pub struct Prompt {
|
||||
@@ -33,6 +78,10 @@ pub struct Prompt {
|
||||
/// Whether to store response on server side (disable_response_storage = !store).
|
||||
pub store: bool,
|
||||
|
||||
/// A list of key-value pairs that will be added as a developer message
|
||||
/// for the model to use
|
||||
pub environment_context: Option<EnvironmentContext>,
|
||||
|
||||
/// Tools available to the model, including additional tools sourced from
|
||||
/// external MCP servers.
|
||||
pub tools: Vec<OpenAiTool>,
|
||||
@@ -54,11 +103,37 @@ impl Prompt {
|
||||
Cow::Owned(sections.join("\n"))
|
||||
}
|
||||
|
||||
pub(crate) fn get_formatted_user_instructions(&self) -> Option<String> {
|
||||
fn get_formatted_user_instructions(&self) -> Option<String> {
|
||||
self.user_instructions
|
||||
.as_ref()
|
||||
.map(|ui| format!("{USER_INSTRUCTIONS_START}{ui}{USER_INSTRUCTIONS_END}"))
|
||||
}
|
||||
|
||||
fn get_formatted_environment_context(&self) -> Option<String> {
|
||||
self.environment_context
|
||||
.as_ref()
|
||||
.map(|ec| format!("{ENVIRONMENT_CONTEXT_START}{ec}{ENVIRONMENT_CONTEXT_END}"))
|
||||
}
|
||||
|
||||
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
|
||||
let mut input_with_instructions = Vec::with_capacity(self.input.len() + 2);
|
||||
if let Some(ec) = self.get_formatted_environment_context() {
|
||||
input_with_instructions.push(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: ec }],
|
||||
});
|
||||
}
|
||||
if let Some(ui) = self.get_formatted_user_instructions() {
|
||||
input_with_instructions.push(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: ui }],
|
||||
});
|
||||
}
|
||||
input_with_instructions.extend(self.input.clone());
|
||||
input_with_instructions
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
||||
@@ -37,6 +37,7 @@ use crate::apply_patch::convert_apply_patch_to_protocol;
|
||||
use crate::apply_patch::get_writable_roots;
|
||||
use crate::apply_patch::{self};
|
||||
use crate::client::ModelClient;
|
||||
use crate::client_common::EnvironmentContext;
|
||||
use crate::client_common::Prompt;
|
||||
use crate::client_common::ResponseEvent;
|
||||
use crate::config::Config;
|
||||
@@ -51,6 +52,8 @@ use crate::exec::SandboxType;
|
||||
use crate::exec::StdoutStream;
|
||||
use crate::exec::process_exec_tool_call;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::git_info::GitInfo;
|
||||
use crate::git_info::collect_git_info;
|
||||
use crate::mcp_connection_manager::McpConnectionManager;
|
||||
use crate::mcp_tool_call::handle_mcp_tool_call;
|
||||
use crate::models::ContentItem;
|
||||
@@ -211,6 +214,7 @@ pub(crate) struct Session {
|
||||
/// the model as well as sandbox policies are resolved against this path
|
||||
/// instead of `std::env::current_dir()`.
|
||||
pub(crate) cwd: PathBuf,
|
||||
git_info: Option<GitInfo>,
|
||||
base_instructions: Option<String>,
|
||||
user_instructions: Option<String>,
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
@@ -723,11 +727,12 @@ async fn submission_loop(
|
||||
}
|
||||
return;
|
||||
}
|
||||
let git_info = collect_git_info(&cwd).await;
|
||||
// Optionally resume an existing rollout.
|
||||
let mut restored_items: Option<Vec<ResponseItem>> = None;
|
||||
let rollout_recorder: Option<RolloutRecorder> =
|
||||
if let Some(path) = resume_path.as_ref() {
|
||||
match RolloutRecorder::resume(path, cwd.clone()).await {
|
||||
match RolloutRecorder::resume(path, git_info.clone()).await {
|
||||
Ok((rec, saved)) => {
|
||||
session_id = saved.session_id;
|
||||
if !saved.items.is_empty() {
|
||||
@@ -747,8 +752,13 @@ async fn submission_loop(
|
||||
let rollout_recorder = match rollout_recorder {
|
||||
Some(rec) => Some(rec),
|
||||
None => {
|
||||
match RolloutRecorder::new(&config, session_id, user_instructions.clone())
|
||||
.await
|
||||
match RolloutRecorder::new(
|
||||
&config,
|
||||
session_id,
|
||||
user_instructions.clone(),
|
||||
git_info.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(r) => Some(r),
|
||||
Err(e) => {
|
||||
@@ -827,6 +837,7 @@ async fn submission_loop(
|
||||
sandbox_policy,
|
||||
shell_environment_policy: config.shell_environment_policy.clone(),
|
||||
cwd,
|
||||
git_info,
|
||||
writable_roots,
|
||||
mcp_connection_manager,
|
||||
notify,
|
||||
@@ -1224,6 +1235,12 @@ async fn run_turn(
|
||||
store: !sess.disable_response_storage,
|
||||
tools,
|
||||
base_instructions_override: sess.base_instructions.clone(),
|
||||
environment_context: Some(EnvironmentContext {
|
||||
cwd: sess.cwd.clone(),
|
||||
git_info: sess.git_info.clone(),
|
||||
approval_policy: sess.approval_policy,
|
||||
sandbox_policy: sess.sandbox_policy.clone(),
|
||||
}),
|
||||
};
|
||||
|
||||
let mut retries = 0;
|
||||
@@ -1449,6 +1466,7 @@ async fn run_compact_task(
|
||||
input: turn_input,
|
||||
user_instructions: None,
|
||||
store: !sess.disable_response_storage,
|
||||
environment_context: None,
|
||||
tools: Vec::new(),
|
||||
base_instructions_override: Some(compact_instructions.clone()),
|
||||
};
|
||||
|
||||
@@ -9,7 +9,7 @@ use tokio::time::timeout;
|
||||
/// Timeout for git commands to prevent freezing on large repositories
|
||||
const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5);
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone)]
|
||||
#[derive(Serialize, Deserialize, Clone, Debug)]
|
||||
pub struct GitInfo {
|
||||
/// Current commit hash (SHA)
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -159,7 +159,8 @@ pub enum AskForApproval {
|
||||
}
|
||||
|
||||
/// Determines execution restrictions for model shell commands.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display)]
|
||||
#[strum(serialize_all = "kebab-case")]
|
||||
#[serde(tag = "mode", rename_all = "kebab-case")]
|
||||
pub enum SandboxPolicy {
|
||||
/// No restrictions whatsoever. Use with caution.
|
||||
|
||||
@@ -21,7 +21,6 @@ use uuid::Uuid;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::git_info::GitInfo;
|
||||
use crate::git_info::collect_git_info;
|
||||
use crate::models::ResponseItem;
|
||||
|
||||
const SESSIONS_SUBDIR: &str = "sessions";
|
||||
@@ -82,6 +81,7 @@ impl RolloutRecorder {
|
||||
config: &Config,
|
||||
uuid: Uuid,
|
||||
instructions: Option<String>,
|
||||
git_info: Option<GitInfo>,
|
||||
) -> std::io::Result<Self> {
|
||||
let LogFileInfo {
|
||||
file,
|
||||
@@ -96,9 +96,6 @@ impl RolloutRecorder {
|
||||
.format(timestamp_format)
|
||||
.map_err(|e| IoError::other(format!("failed to format timestamp: {e}")))?;
|
||||
|
||||
// Clone the cwd for the spawned task to collect git info asynchronously
|
||||
let cwd = config.cwd.clone();
|
||||
|
||||
// A reasonably-sized bounded channel. If the buffer fills up the send
|
||||
// future will yield, which is fine – we only need to ensure we do not
|
||||
// perform *blocking* I/O on the caller's thread.
|
||||
@@ -115,7 +112,7 @@ impl RolloutRecorder {
|
||||
id: session_id,
|
||||
instructions,
|
||||
}),
|
||||
cwd,
|
||||
git_info,
|
||||
));
|
||||
|
||||
Ok(Self { tx })
|
||||
@@ -157,7 +154,7 @@ impl RolloutRecorder {
|
||||
|
||||
pub async fn resume(
|
||||
path: &Path,
|
||||
cwd: std::path::PathBuf,
|
||||
git_info: Option<GitInfo>,
|
||||
) -> std::io::Result<(Self, SavedSession)> {
|
||||
info!("Resuming rollout from {path:?}");
|
||||
let text = tokio::fs::read_to_string(path).await?;
|
||||
@@ -220,7 +217,7 @@ impl RolloutRecorder {
|
||||
tokio::fs::File::from_std(file),
|
||||
rx,
|
||||
None,
|
||||
cwd,
|
||||
git_info,
|
||||
));
|
||||
info!("Resumed rollout successfully from {path:?}");
|
||||
Ok((Self { tx }, saved))
|
||||
@@ -291,13 +288,12 @@ async fn rollout_writer(
|
||||
file: tokio::fs::File,
|
||||
mut rx: mpsc::Receiver<RolloutCmd>,
|
||||
mut meta: Option<SessionMeta>,
|
||||
cwd: std::path::PathBuf,
|
||||
git_info: Option<GitInfo>,
|
||||
) -> std::io::Result<()> {
|
||||
let mut writer = JsonlWriter { file };
|
||||
|
||||
// If we have a meta, collect git info asynchronously and write meta first
|
||||
if let Some(session_meta) = meta.take() {
|
||||
let git_info = collect_git_info(&cwd).await;
|
||||
let session_meta_with_git = SessionMetaWithGit {
|
||||
meta: session_meta,
|
||||
git: git_info,
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
#![allow(clippy::expect_used)]
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use std::path::PathBuf;
|
||||
|
||||
use chrono::Utc;
|
||||
@@ -32,6 +34,32 @@ fn sse_completed(id: &str) -> String {
|
||||
load_sse_fixture_with_id("tests/fixtures/completed_template.json", id)
|
||||
}
|
||||
|
||||
fn assert_message_role(request_body: &serde_json::Value, role: &str) {
|
||||
assert_eq!(request_body["role"].as_str().unwrap(), role);
|
||||
}
|
||||
|
||||
fn assert_message_starts_with(request_body: &serde_json::Value, text: &str) {
|
||||
let content = request_body["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("invalid message content");
|
||||
|
||||
assert!(
|
||||
content.starts_with(text),
|
||||
"expected message content '{content}' to start with '{text}'"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_message_ends_with(request_body: &serde_json::Value, text: &str) {
|
||||
let content = request_body["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("invalid message content");
|
||||
|
||||
assert!(
|
||||
content.ends_with(text),
|
||||
"expected message content '{content}' to end with '{text}'"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn includes_session_id_and_model_headers_in_request() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
@@ -371,19 +399,12 @@ async fn includes_user_instructions_message_in_request() {
|
||||
.unwrap()
|
||||
.contains("be nice")
|
||||
);
|
||||
assert_eq!(request_body["input"][0]["role"], "user");
|
||||
assert!(
|
||||
request_body["input"][0]["content"][0]["text"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.starts_with("<user_instructions>\n\nbe nice")
|
||||
);
|
||||
assert!(
|
||||
request_body["input"][0]["content"][0]["text"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.ends_with("</user_instructions>")
|
||||
);
|
||||
assert_message_role(&request_body["input"][0], "user");
|
||||
assert_message_starts_with(&request_body["input"][0], "<environment_context>\n\n");
|
||||
assert_message_ends_with(&request_body["input"][0], "</environment_context>");
|
||||
assert_message_role(&request_body["input"][1], "user");
|
||||
assert_message_starts_with(&request_body["input"][1], "<user_instructions>\n\n");
|
||||
assert_message_ends_with(&request_body["input"][1], "</user_instructions>");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
use std::path::Path;
|
||||
|
||||
use codex_common::summarize_sandbox_policy;
|
||||
use codex_core::WireApi;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::Event;
|
||||
|
||||
@@ -19,30 +17,6 @@ pub(crate) trait EventProcessor {
|
||||
fn process_event(&mut self, event: Event) -> CodexStatus;
|
||||
}
|
||||
|
||||
pub(crate) fn create_config_summary_entries(config: &Config) -> Vec<(&'static str, String)> {
|
||||
let mut entries = vec![
|
||||
("workdir", config.cwd.display().to_string()),
|
||||
("model", config.model.clone()),
|
||||
("provider", config.model_provider_id.clone()),
|
||||
("approval", config.approval_policy.to_string()),
|
||||
("sandbox", summarize_sandbox_policy(&config.sandbox_policy)),
|
||||
];
|
||||
if config.model_provider.wire_api == WireApi::Responses
|
||||
&& config.model_family.supports_reasoning_summaries
|
||||
{
|
||||
entries.push((
|
||||
"reasoning effort",
|
||||
config.model_reasoning_effort.to_string(),
|
||||
));
|
||||
entries.push((
|
||||
"reasoning summaries",
|
||||
config.model_reasoning_summary.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
entries
|
||||
}
|
||||
|
||||
pub(crate) fn handle_last_message(last_agent_message: Option<&str>, output_file: &Path) {
|
||||
let message = last_agent_message.unwrap_or_default();
|
||||
write_last_message_file(message, Some(output_file));
|
||||
|
||||
@@ -33,8 +33,8 @@ use std::time::Instant;
|
||||
|
||||
use crate::event_processor::CodexStatus;
|
||||
use crate::event_processor::EventProcessor;
|
||||
use crate::event_processor::create_config_summary_entries;
|
||||
use crate::event_processor::handle_last_message;
|
||||
use codex_common::create_config_summary_entries;
|
||||
|
||||
/// This should be configurable. When used in CI, users may not want to impose
|
||||
/// a limit so they can see the full transcript.
|
||||
|
||||
@@ -9,8 +9,8 @@ use serde_json::json;
|
||||
|
||||
use crate::event_processor::CodexStatus;
|
||||
use crate::event_processor::EventProcessor;
|
||||
use crate::event_processor::create_config_summary_entries;
|
||||
use crate::event_processor::handle_last_message;
|
||||
use codex_common::create_config_summary_entries;
|
||||
|
||||
pub(crate) struct EventProcessorWithJsonOutput {
|
||||
last_message_path: Option<PathBuf>,
|
||||
|
||||
@@ -18,7 +18,7 @@ use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[tokio::test]
|
||||
async fn test_send_message_success() {
|
||||
// Spin up a mock completions server that immediately ends the Codex turn.
|
||||
// Two Codex turns hit the mock model (session start + send-user-message). Provide two SSE responses.
|
||||
@@ -99,13 +99,13 @@ async fn test_send_message_success() {
|
||||
response
|
||||
);
|
||||
// wait for the server to hear the user message
|
||||
sleep(Duration::from_secs(1));
|
||||
sleep(Duration::from_secs(10));
|
||||
|
||||
// Ensure the server and tempdir live until end of test
|
||||
drop(server);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[tokio::test]
|
||||
async fn test_send_message_session_not_found() {
|
||||
// Start MCP without creating a Codex session
|
||||
let codex_home = TempDir::new().expect("tempdir");
|
||||
|
||||
@@ -19,7 +19,33 @@ if ! git diff --quiet || ! git diff --cached --quiet || [ -n "$(git ls-files --o
|
||||
fi
|
||||
|
||||
# Fail if in a detached HEAD state.
|
||||
CURRENT_BRANCH=$(git symbolic-ref --short -q HEAD)
|
||||
CURRENT_BRANCH=$(git symbolic-ref --short -q HEAD 2>/dev/null || true)
|
||||
if [ -z "${CURRENT_BRANCH:-}" ]; then
|
||||
echo "ERROR: Could not determine the current branch (detached HEAD?)." >&2
|
||||
echo " Please run this script from a checked-out branch." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Ensure we are on the 'main' branch before proceeding.
|
||||
if [ "${CURRENT_BRANCH}" != "main" ]; then
|
||||
echo "ERROR: Releases must be created from the 'main' branch (current: '${CURRENT_BRANCH}')." >&2
|
||||
echo " Please switch to 'main' and try again." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Ensure the current local commit on 'main' is present on 'origin/main'.
|
||||
# This guarantees we only create releases from commits that are already on
|
||||
# the canonical repository (https://github.com/openai/codex).
|
||||
if ! git fetch --quiet origin main; then
|
||||
echo "ERROR: Failed to fetch 'origin/main'. Ensure the 'origin' remote is configured and reachable." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if ! git merge-base --is-ancestor HEAD origin/main; then
|
||||
echo "ERROR: Your local 'main' HEAD commit is not present on 'origin/main'." >&2
|
||||
echo " Please push your commits first (git push origin main) or check out a commit on 'origin/main'." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Create a new branch for the release and make a commit with the new version.
|
||||
if [ $# -ge 1 ]; then
|
||||
|
||||
@@ -330,6 +330,11 @@ impl App<'_> {
|
||||
widget.add_diff_output(text);
|
||||
}
|
||||
}
|
||||
SlashCommand::Status => {
|
||||
if let AppState::Chat { widget } = &mut self.app_state {
|
||||
widget.add_status_output();
|
||||
}
|
||||
}
|
||||
#[cfg(debug_assertions)]
|
||||
SlashCommand::TestApproval => {
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -522,6 +522,13 @@ impl ChatWidget<'_> {
|
||||
self.add_to_history(HistoryCell::new_diff_output(diff_output.clone()));
|
||||
}
|
||||
|
||||
pub(crate) fn add_status_output(&mut self) {
|
||||
self.add_to_history(HistoryCell::new_status_output(
|
||||
&self.config,
|
||||
&self.token_usage,
|
||||
));
|
||||
}
|
||||
|
||||
/// Forward file-search results to the bottom pane.
|
||||
pub(crate) fn apply_file_search_result(&mut self, query: String, matches: Vec<FileMatch>) {
|
||||
self.bottom_pane.on_file_search_result(query, matches);
|
||||
|
||||
@@ -3,9 +3,8 @@ use crate::text_block::TextBlock;
|
||||
use crate::text_formatting::format_and_truncate_tool_result;
|
||||
use base64::Engine;
|
||||
use codex_ansi_escape::ansi_escape_line;
|
||||
use codex_common::create_config_summary_entries;
|
||||
use codex_common::elapsed::format_duration;
|
||||
use codex_common::summarize_sandbox_policy;
|
||||
use codex_core::WireApi;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::plan_tool::PlanItemArg;
|
||||
use codex_core::plan_tool::StepStatus;
|
||||
@@ -14,6 +13,7 @@ use codex_core::protocol::FileChange;
|
||||
use codex_core::protocol::McpInvocation;
|
||||
use codex_core::protocol::PatchApplyEndEvent;
|
||||
use codex_core::protocol::SessionConfiguredEvent;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use image::DynamicImage;
|
||||
use image::ImageReader;
|
||||
use mcp_types::EmbeddedResourceResource;
|
||||
@@ -28,7 +28,6 @@ use std::collections::HashMap;
|
||||
use std::io::Cursor;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
use textwrap::Options;
|
||||
use tracing::error;
|
||||
|
||||
pub(crate) struct CommandOutput {
|
||||
@@ -115,6 +114,11 @@ pub(crate) enum HistoryCell {
|
||||
view: TextBlock,
|
||||
},
|
||||
|
||||
/// Output from the `/status` command.
|
||||
StatusOutput {
|
||||
view: TextBlock,
|
||||
},
|
||||
|
||||
/// Error event from the backend.
|
||||
ErrorEvent {
|
||||
view: TextBlock,
|
||||
@@ -155,6 +159,7 @@ impl HistoryCell {
|
||||
| HistoryCell::UserPrompt { view }
|
||||
| HistoryCell::BackgroundEvent { view }
|
||||
| HistoryCell::GitDiffOutput { view }
|
||||
| HistoryCell::StatusOutput { view }
|
||||
| HistoryCell::ErrorEvent { view }
|
||||
| HistoryCell::SessionInfo { view }
|
||||
| HistoryCell::CompletedExecCommand { view }
|
||||
@@ -201,26 +206,7 @@ impl HistoryCell {
|
||||
]),
|
||||
];
|
||||
|
||||
let mut entries = vec![
|
||||
("workdir", config.cwd.display().to_string()),
|
||||
("model", config.model.clone()),
|
||||
("provider", config.model_provider_id.clone()),
|
||||
("approval", config.approval_policy.to_string()),
|
||||
("sandbox", summarize_sandbox_policy(&config.sandbox_policy)),
|
||||
];
|
||||
if config.model_provider.wire_api == WireApi::Responses
|
||||
&& config.model_family.supports_reasoning_summaries
|
||||
{
|
||||
entries.push((
|
||||
"reasoning effort",
|
||||
config.model_reasoning_effort.to_string(),
|
||||
));
|
||||
entries.push((
|
||||
"reasoning summaries",
|
||||
config.model_reasoning_summary.to_string(),
|
||||
));
|
||||
}
|
||||
for (key, value) in entries {
|
||||
for (key, value) in create_config_summary_entries(config) {
|
||||
lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()]));
|
||||
}
|
||||
lines.push(Line::from(""));
|
||||
@@ -247,15 +233,7 @@ impl HistoryCell {
|
||||
pub(crate) fn new_user_prompt(message: String) -> Self {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
lines.push(Line::from("user".cyan().bold()));
|
||||
let width = 80;
|
||||
lines.extend(
|
||||
textwrap::wrap(
|
||||
&message,
|
||||
Options::new(width).wrap_algorithm(textwrap::WrapAlgorithm::FirstFit),
|
||||
)
|
||||
.iter()
|
||||
.map(|l| Line::from(l.to_string())),
|
||||
);
|
||||
lines.extend(message.lines().map(|l| Line::from(l.to_string())));
|
||||
lines.push(Line::from(""));
|
||||
|
||||
HistoryCell::UserPrompt {
|
||||
@@ -485,6 +463,49 @@ impl HistoryCell {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn new_status_output(config: &Config, usage: &TokenUsage) -> Self {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
lines.push(Line::from("/status".magenta()));
|
||||
|
||||
// Config
|
||||
for (key, value) in create_config_summary_entries(config) {
|
||||
lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()]));
|
||||
}
|
||||
|
||||
// Token usage
|
||||
lines.push(Line::from(""));
|
||||
lines.push(Line::from("token usage".bold()));
|
||||
lines.push(Line::from(vec![
|
||||
" input: ".bold(),
|
||||
usage.input_tokens.to_string().into(),
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
" cached input: ".bold(),
|
||||
usage.cached_input_tokens.unwrap_or(0).to_string().into(),
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
" output: ".bold(),
|
||||
usage.output_tokens.to_string().into(),
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
" reasoning output: ".bold(),
|
||||
usage
|
||||
.reasoning_output_tokens
|
||||
.unwrap_or(0)
|
||||
.to_string()
|
||||
.into(),
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
" total: ".bold(),
|
||||
usage.total_tokens.to_string().into(),
|
||||
]));
|
||||
|
||||
lines.push(Line::from(""));
|
||||
HistoryCell::StatusOutput {
|
||||
view: TextBlock::new(lines),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn new_error_event(message: String) -> Self {
|
||||
let lines: Vec<Line<'static>> = vec![
|
||||
vec!["ERROR: ".red().bold(), message.into()].into(),
|
||||
|
||||
@@ -15,6 +15,7 @@ pub enum SlashCommand {
|
||||
New,
|
||||
Compact,
|
||||
Diff,
|
||||
Status,
|
||||
Quit,
|
||||
#[cfg(debug_assertions)]
|
||||
TestApproval,
|
||||
@@ -24,12 +25,11 @@ impl SlashCommand {
|
||||
/// User-visible description shown in the popup.
|
||||
pub fn description(self) -> &'static str {
|
||||
match self {
|
||||
SlashCommand::New => "Start a new chat.",
|
||||
SlashCommand::Compact => "Compact the chat history.",
|
||||
SlashCommand::Quit => "Exit the application.",
|
||||
SlashCommand::Diff => {
|
||||
"Show git diff of the working directory (including untracked files)"
|
||||
}
|
||||
SlashCommand::New => "Start a new chat",
|
||||
SlashCommand::Compact => "Compact the chat history",
|
||||
SlashCommand::Quit => "Exit the application",
|
||||
SlashCommand::Diff => "Show git diff (including untracked files)",
|
||||
SlashCommand::Status => "Show current session configuration and token usage",
|
||||
#[cfg(debug_assertions)]
|
||||
SlashCommand::TestApproval => "Test approval request",
|
||||
}
|
||||
|
||||
@@ -39,10 +39,7 @@ pub fn init(_config: &Config) -> Result<Tui> {
|
||||
)?;
|
||||
set_panic_hook();
|
||||
|
||||
// Ensure the UI starts at the top of the terminal by clearing the
|
||||
// current screen and moving the cursor to (0, 0) before creating the
|
||||
// Terminal. This makes the initial welcome message render at the very top
|
||||
// of the viewport, while keeping the normal scrollback history intact.
|
||||
// Clear screen and move cursor to top-left before drawing UI
|
||||
execute!(stdout(), Clear(ClearType::All), MoveTo(0, 0))?;
|
||||
|
||||
let backend = CrosstermBackend::new(stdout());
|
||||
|
||||
Reference in New Issue
Block a user