mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Merge ee7c160256 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -369,6 +369,20 @@ jobs:
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
|
||||
# Some integration tests rely on DotSlash being installed.
|
||||
# See https://github.com/openai/codex/pull/7617.
|
||||
- name: Install DotSlash
|
||||
uses: facebook/install-dotslash@v2
|
||||
|
||||
- name: Pre-fetch DotSlash artifacts
|
||||
# The Bash wrapper is not available on Windows.
|
||||
if: ${{ !startsWith(matrix.runner, 'windows') }}
|
||||
shell: bash
|
||||
run: |
|
||||
set -euo pipefail
|
||||
dotslash -- fetch exec-server/tests/suite/bash
|
||||
|
||||
- uses: dtolnay/rust-toolchain@1.90
|
||||
with:
|
||||
targets: ${{ matrix.target }}
|
||||
|
||||
@@ -75,6 +75,7 @@ If you don’t have the tool:
|
||||
### Test assertions
|
||||
|
||||
- Tests should use pretty_assertions::assert_eq for clearer diffs. Import this at the top of the test module if it isn't already.
|
||||
- Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields.
|
||||
|
||||
### Integration tests (core)
|
||||
|
||||
|
||||
Generated
+39
-8
@@ -1048,7 +1048,7 @@ dependencies = [
|
||||
"pretty_assertions",
|
||||
"regex-lite",
|
||||
"serde_json",
|
||||
"supports-color",
|
||||
"supports-color 3.0.2",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"toml",
|
||||
@@ -1088,10 +1088,13 @@ dependencies = [
|
||||
"codex-login",
|
||||
"codex-tui",
|
||||
"crossterm",
|
||||
"owo-colors",
|
||||
"pretty_assertions",
|
||||
"ratatui",
|
||||
"reqwest",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"supports-color 3.0.2",
|
||||
"tokio",
|
||||
"tokio-stream",
|
||||
"tracing",
|
||||
@@ -1237,7 +1240,7 @@ dependencies = [
|
||||
"serde",
|
||||
"serde_json",
|
||||
"shlex",
|
||||
"supports-color",
|
||||
"supports-color 3.0.2",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"tracing",
|
||||
@@ -1253,11 +1256,14 @@ name = "codex-exec-server"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"assert_cmd",
|
||||
"async-trait",
|
||||
"clap",
|
||||
"codex-core",
|
||||
"codex-execpolicy",
|
||||
"exec_server_test_support",
|
||||
"libc",
|
||||
"maplit",
|
||||
"path-absolutize",
|
||||
"pretty_assertions",
|
||||
"rmcp",
|
||||
@@ -1270,6 +1276,7 @@ dependencies = [
|
||||
"tokio-util",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
"which",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1611,7 +1618,7 @@ dependencies = [
|
||||
"shlex",
|
||||
"strum 0.27.2",
|
||||
"strum_macros 0.27.2",
|
||||
"supports-color",
|
||||
"supports-color 3.0.2",
|
||||
"tempfile",
|
||||
"textwrap 0.16.2",
|
||||
"tokio",
|
||||
@@ -2498,6 +2505,18 @@ dependencies = [
|
||||
"pin-project-lite",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "exec_server_test_support"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"assert_cmd",
|
||||
"codex-core",
|
||||
"rmcp",
|
||||
"serde_json",
|
||||
"tokio",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "eyre"
|
||||
version = "0.6.12"
|
||||
@@ -2557,8 +2576,7 @@ dependencies = [
|
||||
[[package]]
|
||||
name = "filedescriptor"
|
||||
version = "0.8.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e40758ed24c9b2eeb76c35fb0aebc66c626084edd827e07e1552279814c6682d"
|
||||
source = "git+https://github.com/pakrym/wezterm?branch=PSUEDOCONSOLE_INHERIT_CURSOR#fe38df8409545a696909aa9a09e63438630f217d"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"thiserror 1.0.69",
|
||||
@@ -4434,6 +4452,10 @@ name = "owo-colors"
|
||||
version = "4.2.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "48dd4f4a2c8405440fd0462561f0e5806bd0f77e86f51c761481bdd4018b545e"
|
||||
dependencies = [
|
||||
"supports-color 2.1.0",
|
||||
"supports-color 3.0.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "parking"
|
||||
@@ -4632,8 +4654,7 @@ dependencies = [
|
||||
[[package]]
|
||||
name = "portable-pty"
|
||||
version = "0.9.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b4a596a2b3d2752d94f51fac2d4a96737b8705dddd311a32b9af47211f08671e"
|
||||
source = "git+https://github.com/pakrym/wezterm?branch=PSUEDOCONSOLE_INHERIT_CURSOR#fe38df8409545a696909aa9a09e63438630f217d"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"bitflags 1.3.2",
|
||||
@@ -4642,7 +4663,7 @@ dependencies = [
|
||||
"lazy_static",
|
||||
"libc",
|
||||
"log",
|
||||
"nix 0.28.0",
|
||||
"nix 0.29.0",
|
||||
"serial2",
|
||||
"shared_library",
|
||||
"shell-words",
|
||||
@@ -6170,6 +6191,16 @@ version = "2.6.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292"
|
||||
|
||||
[[package]]
|
||||
name = "supports-color"
|
||||
version = "2.1.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "d6398cde53adc3c4557306a96ce67b302968513830a77a95b2b17305d9719a89"
|
||||
dependencies = [
|
||||
"is-terminal",
|
||||
"is_ci",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "supports-color"
|
||||
version = "3.0.2"
|
||||
|
||||
+3
-1
@@ -96,6 +96,7 @@ codex-utils-readiness = { path = "utils/readiness" }
|
||||
codex-utils-string = { path = "utils/string" }
|
||||
codex-windows-sandbox = { path = "windows-sandbox-rs" }
|
||||
core_test_support = { path = "core/tests/common" }
|
||||
exec_server_test_support = { path = "exec-server/tests/common" }
|
||||
mcp-types = { path = "mcp-types" }
|
||||
mcp_test_support = { path = "mcp-server/tests/common" }
|
||||
|
||||
@@ -178,8 +179,8 @@ seccompiler = "0.5.0"
|
||||
sentry = "0.34.0"
|
||||
serde = "1"
|
||||
serde_json = "1"
|
||||
serde_yaml = "0.9"
|
||||
serde_with = "3.16"
|
||||
serde_yaml = "0.9"
|
||||
serial_test = "3.2.0"
|
||||
sha1 = "0.10.6"
|
||||
sha2 = "0.10"
|
||||
@@ -288,6 +289,7 @@ opt-level = 0
|
||||
# Uncomment to debug local changes.
|
||||
# ratatui = { path = "../../ratatui" }
|
||||
crossterm = { git = "https://github.com/nornagon/crossterm", branch = "nornagon/color-query" }
|
||||
portable-pty = { git = "https://github.com/pakrym/wezterm", branch = "PSUEDOCONSOLE_INHERIT_CURSOR" }
|
||||
ratatui = { git = "https://github.com/nornagon/ratatui", branch = "nornagon-v0.29.0-patch" }
|
||||
|
||||
# Uncomment to debug local changes.
|
||||
|
||||
@@ -699,13 +699,7 @@ fn derive_new_contents_from_chunks(
|
||||
}
|
||||
};
|
||||
|
||||
let mut original_lines: Vec<String> = original_contents.split('\n').map(String::from).collect();
|
||||
|
||||
// Drop the trailing empty element that results from the final newline so
|
||||
// that line counts match the behaviour of standard `diff`.
|
||||
if original_lines.last().is_some_and(String::is_empty) {
|
||||
original_lines.pop();
|
||||
}
|
||||
let original_lines: Vec<String> = build_lines_from_contents(&original_contents);
|
||||
|
||||
let replacements = compute_replacements(&original_lines, path, chunks)?;
|
||||
let new_lines = apply_replacements(original_lines, &replacements);
|
||||
@@ -713,13 +707,67 @@ fn derive_new_contents_from_chunks(
|
||||
if !new_lines.last().is_some_and(String::is_empty) {
|
||||
new_lines.push(String::new());
|
||||
}
|
||||
let new_contents = new_lines.join("\n");
|
||||
let new_contents = build_contents_from_lines(&original_contents, &new_lines);
|
||||
Ok(AppliedPatch {
|
||||
original_contents,
|
||||
new_contents,
|
||||
})
|
||||
}
|
||||
|
||||
// TODO(dylan-hurd-oai): I think we can migrate to just use `contents.lines()`
|
||||
// across all platforms.
|
||||
fn build_lines_from_contents(contents: &str) -> Vec<String> {
|
||||
if cfg!(windows) {
|
||||
contents.lines().map(String::from).collect()
|
||||
} else {
|
||||
let mut lines: Vec<String> = contents.split('\n').map(String::from).collect();
|
||||
|
||||
// Drop the trailing empty element that results from the final newline so
|
||||
// that line counts match the behaviour of standard `diff`.
|
||||
if lines.last().is_some_and(String::is_empty) {
|
||||
lines.pop();
|
||||
}
|
||||
|
||||
lines
|
||||
}
|
||||
}
|
||||
|
||||
fn build_contents_from_lines(original_contents: &str, lines: &[String]) -> String {
|
||||
if cfg!(windows) {
|
||||
// for now, only compute this if we're on Windows.
|
||||
let uses_crlf = contents_uses_crlf(original_contents);
|
||||
if uses_crlf {
|
||||
lines.join("\r\n")
|
||||
} else {
|
||||
lines.join("\n")
|
||||
}
|
||||
} else {
|
||||
lines.join("\n")
|
||||
}
|
||||
}
|
||||
|
||||
/// Detects whether the source file uses Windows CRLF line endings consistently.
|
||||
/// We only consider a file CRLF-formatted if every newline is part of a
|
||||
/// CRLF sequence. This avoids rewriting an LF-formatted file that merely
|
||||
/// contains embedded sequences of "\r\n".
|
||||
///
|
||||
/// Returns `true` if the file uses CRLF line endings, `false` otherwise.
|
||||
fn contents_uses_crlf(contents: &str) -> bool {
|
||||
let bytes = contents.as_bytes();
|
||||
let mut n_newlines = 0usize;
|
||||
let mut n_crlf = 0usize;
|
||||
for i in 0..bytes.len() {
|
||||
if bytes[i] == b'\n' {
|
||||
n_newlines += 1;
|
||||
if i > 0 && bytes[i - 1] == b'\r' {
|
||||
n_crlf += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
n_newlines > 0 && n_crlf == n_newlines
|
||||
}
|
||||
|
||||
/// Compute a list of replacements needed to transform `original_lines` into the
|
||||
/// new lines, given the patch `chunks`. Each replacement is returned as
|
||||
/// `(start_index, old_len, new_lines)`.
|
||||
@@ -1359,6 +1407,72 @@ PATCH"#,
|
||||
assert_eq!(contents, "a\nB\nc\nd\nE\nf\ng\n");
|
||||
}
|
||||
|
||||
/// Ensure CRLF line endings are preserved for updated files on Windows‑style inputs.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_preserve_crlf_line_endings_on_update() {
|
||||
let dir = tempdir().unwrap();
|
||||
let path = dir.path().join("crlf.txt");
|
||||
|
||||
// Original file uses CRLF (\r\n) endings.
|
||||
std::fs::write(&path, b"a\r\nb\r\nc\r\n").unwrap();
|
||||
|
||||
// Replace `b` -> `B` and append `d`.
|
||||
let patch = wrap_patch(&format!(
|
||||
r#"*** Update File: {}
|
||||
@@
|
||||
a
|
||||
-b
|
||||
+B
|
||||
@@
|
||||
c
|
||||
+d
|
||||
*** End of File"#,
|
||||
path.display()
|
||||
));
|
||||
|
||||
let mut stdout = Vec::new();
|
||||
let mut stderr = Vec::new();
|
||||
apply_patch(&patch, &mut stdout, &mut stderr).unwrap();
|
||||
|
||||
let out = std::fs::read(&path).unwrap();
|
||||
// Expect all CRLF endings; count occurrences of CRLF and ensure there are 4 lines.
|
||||
let content = String::from_utf8_lossy(&out);
|
||||
assert!(content.contains("\r\n"));
|
||||
// No bare LF occurrences immediately preceding a non-CR: the text should not contain "a\nb".
|
||||
assert!(!content.contains("a\nb"));
|
||||
// Validate exact content sequence with CRLF delimiters.
|
||||
assert_eq!(content, "a\r\nB\r\nc\r\nd\r\n");
|
||||
}
|
||||
|
||||
/// Ensure CRLF inputs with embedded carriage returns in the content are preserved.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_preserve_crlf_embedded_carriage_returns_on_append() {
|
||||
let dir = tempdir().unwrap();
|
||||
let path = dir.path().join("crlf_cr_content.txt");
|
||||
|
||||
// Original file: first line has a literal '\r' in the content before the CRLF terminator.
|
||||
std::fs::write(&path, b"foo\r\r\nbar\r\n").unwrap();
|
||||
|
||||
// Append a new line without modifying existing ones.
|
||||
let patch = wrap_patch(&format!(
|
||||
r#"*** Update File: {}
|
||||
@@
|
||||
+BAZ
|
||||
*** End of File"#,
|
||||
path.display()
|
||||
));
|
||||
|
||||
let mut stdout = Vec::new();
|
||||
let mut stderr = Vec::new();
|
||||
apply_patch(&patch, &mut stdout, &mut stderr).unwrap();
|
||||
|
||||
let out = std::fs::read(&path).unwrap();
|
||||
// CRLF endings must be preserved and the extra CR in "foo\r\r" must not be collapsed.
|
||||
assert_eq!(out.as_slice(), b"foo\r\r\nbar\r\nBAZ\r\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_pure_addition_chunk_followed_by_removal() {
|
||||
let dir = tempdir().unwrap();
|
||||
@@ -1544,6 +1658,37 @@ PATCH"#,
|
||||
assert_eq!(expected, diff);
|
||||
}
|
||||
|
||||
/// For LF-only inputs with a trailing newline ensure that the helper used
|
||||
/// on Windows-style builds drops the synthetic trailing empty element so
|
||||
/// replacements behave like standard `diff` line numbering.
|
||||
#[test]
|
||||
fn test_derive_new_contents_lf_trailing_newline() {
|
||||
let dir = tempdir().unwrap();
|
||||
let path = dir.path().join("lf_trailing_newline.txt");
|
||||
fs::write(&path, "foo\nbar\n").unwrap();
|
||||
|
||||
let patch = wrap_patch(&format!(
|
||||
r#"*** Update File: {}
|
||||
@@
|
||||
foo
|
||||
-bar
|
||||
+BAR
|
||||
"#,
|
||||
path.display()
|
||||
));
|
||||
|
||||
let patch = parse_patch(&patch).unwrap();
|
||||
let chunks = match patch.hunks.as_slice() {
|
||||
[Hunk::UpdateFile { chunks, .. }] => chunks,
|
||||
_ => panic!("Expected a single UpdateFile hunk"),
|
||||
};
|
||||
|
||||
let AppliedPatch { new_contents, .. } =
|
||||
derive_new_contents_from_chunks(&path, chunks).unwrap();
|
||||
|
||||
assert_eq!(new_contents, "foo\nBAR\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_unified_diff_insert_at_eof() {
|
||||
// Insert a new line at end‑of‑file.
|
||||
|
||||
@@ -127,6 +127,7 @@ impl Default for TaskText {
|
||||
#[async_trait::async_trait]
|
||||
pub trait CloudBackend: Send + Sync {
|
||||
async fn list_tasks(&self, env: Option<&str>) -> Result<Vec<TaskSummary>>;
|
||||
async fn get_task_summary(&self, id: TaskId) -> Result<TaskSummary>;
|
||||
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>>;
|
||||
/// Return assistant output messages (no diff) when available.
|
||||
async fn get_task_messages(&self, id: TaskId) -> Result<Vec<String>>;
|
||||
|
||||
@@ -63,6 +63,10 @@ impl CloudBackend for HttpClient {
|
||||
self.tasks_api().list(env).await
|
||||
}
|
||||
|
||||
async fn get_task_summary(&self, id: TaskId) -> Result<TaskSummary> {
|
||||
self.tasks_api().summary(id).await
|
||||
}
|
||||
|
||||
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>> {
|
||||
self.tasks_api().diff(id).await
|
||||
}
|
||||
@@ -149,6 +153,75 @@ mod api {
|
||||
Ok(tasks)
|
||||
}
|
||||
|
||||
pub(crate) async fn summary(&self, id: TaskId) -> Result<TaskSummary> {
|
||||
let id_str = id.0.clone();
|
||||
let (details, body, ct) = self
|
||||
.details_with_body(&id.0)
|
||||
.await
|
||||
.map_err(|e| CloudTaskError::Http(format!("get_task_details failed: {e}")))?;
|
||||
let parsed: Value = serde_json::from_str(&body).map_err(|e| {
|
||||
CloudTaskError::Http(format!(
|
||||
"Decode error for {}: {e}; content-type={ct}; body={body}",
|
||||
id.0
|
||||
))
|
||||
})?;
|
||||
let task_obj = parsed
|
||||
.get("task")
|
||||
.and_then(Value::as_object)
|
||||
.ok_or_else(|| {
|
||||
CloudTaskError::Http(format!("Task metadata missing from details for {id_str}"))
|
||||
})?;
|
||||
let status_display = parsed
|
||||
.get("task_status_display")
|
||||
.or_else(|| task_obj.get("task_status_display"))
|
||||
.and_then(Value::as_object)
|
||||
.map(|m| {
|
||||
m.iter()
|
||||
.map(|(k, v)| (k.clone(), v.clone()))
|
||||
.collect::<HashMap<String, Value>>()
|
||||
});
|
||||
let status = map_status(status_display.as_ref());
|
||||
let mut summary = diff_summary_from_status_display(status_display.as_ref());
|
||||
if summary.files_changed == 0
|
||||
&& summary.lines_added == 0
|
||||
&& summary.lines_removed == 0
|
||||
&& let Some(diff) = details.unified_diff()
|
||||
{
|
||||
summary = diff_summary_from_diff(&diff);
|
||||
}
|
||||
let updated_at_raw = task_obj
|
||||
.get("updated_at")
|
||||
.and_then(Value::as_f64)
|
||||
.or_else(|| task_obj.get("created_at").and_then(Value::as_f64))
|
||||
.or_else(|| latest_turn_timestamp(status_display.as_ref()));
|
||||
let environment_id = task_obj
|
||||
.get("environment_id")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string);
|
||||
let environment_label = env_label_from_status_display(status_display.as_ref());
|
||||
let attempt_total = attempt_total_from_status_display(status_display.as_ref());
|
||||
let title = task_obj
|
||||
.get("title")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("<untitled>")
|
||||
.to_string();
|
||||
let is_review = task_obj
|
||||
.get("is_review")
|
||||
.and_then(Value::as_bool)
|
||||
.unwrap_or(false);
|
||||
Ok(TaskSummary {
|
||||
id,
|
||||
title,
|
||||
status,
|
||||
updated_at: parse_updated_at(updated_at_raw.as_ref()),
|
||||
environment_id,
|
||||
environment_label,
|
||||
summary,
|
||||
is_review,
|
||||
attempt_total,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn diff(&self, id: TaskId) -> Result<Option<String>> {
|
||||
let (details, body, ct) = self
|
||||
.details_with_body(&id.0)
|
||||
@@ -679,6 +752,34 @@ mod api {
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
fn diff_summary_from_diff(diff: &str) -> DiffSummary {
|
||||
let mut files_changed = 0usize;
|
||||
let mut lines_added = 0usize;
|
||||
let mut lines_removed = 0usize;
|
||||
for line in diff.lines() {
|
||||
if line.starts_with("diff --git ") {
|
||||
files_changed += 1;
|
||||
continue;
|
||||
}
|
||||
if line.starts_with("+++") || line.starts_with("---") || line.starts_with("@@") {
|
||||
continue;
|
||||
}
|
||||
match line.as_bytes().first() {
|
||||
Some(b'+') => lines_added += 1,
|
||||
Some(b'-') => lines_removed += 1,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
if files_changed == 0 && !diff.trim().is_empty() {
|
||||
files_changed = 1;
|
||||
}
|
||||
DiffSummary {
|
||||
files_changed,
|
||||
lines_added,
|
||||
lines_removed,
|
||||
}
|
||||
}
|
||||
|
||||
fn diff_summary_from_status_display(v: Option<&HashMap<String, Value>>) -> DiffSummary {
|
||||
let mut out = DiffSummary::default();
|
||||
let Some(map) = v else { return out };
|
||||
@@ -700,6 +801,17 @@ mod api {
|
||||
out
|
||||
}
|
||||
|
||||
fn latest_turn_timestamp(v: Option<&HashMap<String, Value>>) -> Option<f64> {
|
||||
let map = v?;
|
||||
let latest = map
|
||||
.get("latest_turn_status_display")
|
||||
.and_then(Value::as_object)?;
|
||||
latest
|
||||
.get("updated_at")
|
||||
.or_else(|| latest.get("created_at"))
|
||||
.and_then(Value::as_f64)
|
||||
}
|
||||
|
||||
fn attempt_total_from_status_display(v: Option<&HashMap<String, Value>>) -> Option<usize> {
|
||||
let map = v?;
|
||||
let latest = map
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::ApplyOutcome;
|
||||
use crate::AttemptStatus;
|
||||
use crate::CloudBackend;
|
||||
use crate::CloudTaskError;
|
||||
use crate::DiffSummary;
|
||||
use crate::Result;
|
||||
use crate::TaskId;
|
||||
@@ -60,6 +61,14 @@ impl CloudBackend for MockClient {
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
async fn get_task_summary(&self, id: TaskId) -> Result<TaskSummary> {
|
||||
let tasks = self.list_tasks(None).await?;
|
||||
tasks
|
||||
.into_iter()
|
||||
.find(|t| t.id == id)
|
||||
.ok_or_else(|| CloudTaskError::Msg(format!("Task {} not found (mock)", id.0)))
|
||||
}
|
||||
|
||||
async fn get_task_diff(&self, id: TaskId) -> Result<Option<String>> {
|
||||
Ok(Some(mock_diff_for(&id)))
|
||||
}
|
||||
|
||||
@@ -34,6 +34,9 @@ tokio-stream = { workspace = true }
|
||||
tracing = { workspace = true, features = ["log"] }
|
||||
tracing-subscriber = { workspace = true, features = ["env-filter"] }
|
||||
unicode-width = { workspace = true }
|
||||
owo-colors = { workspace = true, features = ["supports-colors"] }
|
||||
supports-color = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
async-trait = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -350,6 +350,7 @@ pub enum AppEvent {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use chrono::Utc;
|
||||
use codex_cloud_tasks_client::CloudTaskError;
|
||||
|
||||
struct FakeBackend {
|
||||
// maps env key to titles
|
||||
@@ -385,6 +386,17 @@ mod tests {
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
async fn get_task_summary(
|
||||
&self,
|
||||
id: TaskId,
|
||||
) -> codex_cloud_tasks_client::Result<TaskSummary> {
|
||||
self.list_tasks(None)
|
||||
.await?
|
||||
.into_iter()
|
||||
.find(|t| t.id == id)
|
||||
.ok_or_else(|| CloudTaskError::Msg(format!("Task {} not found", id.0)))
|
||||
}
|
||||
|
||||
async fn get_task_diff(
|
||||
&self,
|
||||
_id: TaskId,
|
||||
|
||||
@@ -16,6 +16,12 @@ pub struct Cli {
|
||||
pub enum Command {
|
||||
/// Submit a new Codex Cloud task without launching the TUI.
|
||||
Exec(ExecCommand),
|
||||
/// Show the status of a Codex Cloud task.
|
||||
Status(StatusCommand),
|
||||
/// Apply the diff for a Codex Cloud task locally.
|
||||
Apply(ApplyCommand),
|
||||
/// Show the unified diff for a Codex Cloud task.
|
||||
Diff(DiffCommand),
|
||||
}
|
||||
|
||||
#[derive(Debug, Args)]
|
||||
@@ -51,3 +57,32 @@ fn parse_attempts(input: &str) -> Result<usize, String> {
|
||||
Err("attempts must be between 1 and 4".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Args)]
|
||||
pub struct StatusCommand {
|
||||
/// Codex Cloud task identifier to inspect.
|
||||
#[arg(value_name = "TASK_ID")]
|
||||
pub task_id: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Args)]
|
||||
pub struct ApplyCommand {
|
||||
/// Codex Cloud task identifier to apply.
|
||||
#[arg(value_name = "TASK_ID")]
|
||||
pub task_id: String,
|
||||
|
||||
/// Attempt number to apply (1-based).
|
||||
#[arg(long = "attempt", value_parser = parse_attempts, value_name = "N")]
|
||||
pub attempt: Option<usize>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Args)]
|
||||
pub struct DiffCommand {
|
||||
/// Codex Cloud task identifier to display.
|
||||
#[arg(value_name = "TASK_ID")]
|
||||
pub task_id: String,
|
||||
|
||||
/// Attempt number to display (1-based).
|
||||
#[arg(long = "attempt", value_parser = parse_attempts, value_name = "N")]
|
||||
pub attempt: Option<usize>,
|
||||
}
|
||||
|
||||
@@ -8,17 +8,24 @@ pub mod util;
|
||||
pub use cli::Cli;
|
||||
|
||||
use anyhow::anyhow;
|
||||
use chrono::Utc;
|
||||
use codex_cloud_tasks_client::TaskStatus;
|
||||
use codex_login::AuthManager;
|
||||
use owo_colors::OwoColorize;
|
||||
use owo_colors::Stream;
|
||||
use std::cmp::Ordering;
|
||||
use std::io::IsTerminal;
|
||||
use std::io::Read;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
use supports_color::Stream as SupportStream;
|
||||
use tokio::sync::mpsc::UnboundedSender;
|
||||
use tracing::info;
|
||||
use tracing_subscriber::EnvFilter;
|
||||
use util::append_error_log;
|
||||
use util::format_relative_time;
|
||||
use util::set_user_agent_suffix;
|
||||
|
||||
struct ApplyJob {
|
||||
@@ -193,6 +200,273 @@ fn resolve_query_input(query_arg: Option<String>) -> anyhow::Result<String> {
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_task_id(raw: &str) -> anyhow::Result<codex_cloud_tasks_client::TaskId> {
|
||||
let trimmed = raw.trim();
|
||||
if trimmed.is_empty() {
|
||||
anyhow::bail!("task id must not be empty");
|
||||
}
|
||||
let without_fragment = trimmed.split('#').next().unwrap_or(trimmed);
|
||||
let without_query = without_fragment
|
||||
.split('?')
|
||||
.next()
|
||||
.unwrap_or(without_fragment);
|
||||
let id = without_query
|
||||
.rsplit('/')
|
||||
.next()
|
||||
.unwrap_or(without_query)
|
||||
.trim();
|
||||
if id.is_empty() {
|
||||
anyhow::bail!("task id must not be empty");
|
||||
}
|
||||
Ok(codex_cloud_tasks_client::TaskId(id.to_string()))
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
struct AttemptDiffData {
|
||||
placement: Option<i64>,
|
||||
created_at: Option<chrono::DateTime<Utc>>,
|
||||
diff: String,
|
||||
}
|
||||
|
||||
fn cmp_attempt(lhs: &AttemptDiffData, rhs: &AttemptDiffData) -> Ordering {
|
||||
match (lhs.placement, rhs.placement) {
|
||||
(Some(a), Some(b)) => a.cmp(&b),
|
||||
(Some(_), None) => Ordering::Less,
|
||||
(None, Some(_)) => Ordering::Greater,
|
||||
(None, None) => match (lhs.created_at, rhs.created_at) {
|
||||
(Some(a), Some(b)) => a.cmp(&b),
|
||||
(Some(_), None) => Ordering::Less,
|
||||
(None, Some(_)) => Ordering::Greater,
|
||||
(None, None) => Ordering::Equal,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
async fn collect_attempt_diffs(
|
||||
backend: &dyn codex_cloud_tasks_client::CloudBackend,
|
||||
task_id: &codex_cloud_tasks_client::TaskId,
|
||||
) -> anyhow::Result<Vec<AttemptDiffData>> {
|
||||
let text =
|
||||
codex_cloud_tasks_client::CloudBackend::get_task_text(backend, task_id.clone()).await?;
|
||||
let mut attempts = Vec::new();
|
||||
if let Some(diff) =
|
||||
codex_cloud_tasks_client::CloudBackend::get_task_diff(backend, task_id.clone()).await?
|
||||
{
|
||||
attempts.push(AttemptDiffData {
|
||||
placement: text.attempt_placement,
|
||||
created_at: None,
|
||||
diff,
|
||||
});
|
||||
}
|
||||
if let Some(turn_id) = text.turn_id {
|
||||
let siblings = codex_cloud_tasks_client::CloudBackend::list_sibling_attempts(
|
||||
backend,
|
||||
task_id.clone(),
|
||||
turn_id,
|
||||
)
|
||||
.await?;
|
||||
for sibling in siblings {
|
||||
if let Some(diff) = sibling.diff {
|
||||
attempts.push(AttemptDiffData {
|
||||
placement: sibling.attempt_placement,
|
||||
created_at: sibling.created_at,
|
||||
diff,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
attempts.sort_by(cmp_attempt);
|
||||
if attempts.is_empty() {
|
||||
anyhow::bail!(
|
||||
"No diff available for task {}; it may still be running.",
|
||||
task_id.0
|
||||
);
|
||||
}
|
||||
Ok(attempts)
|
||||
}
|
||||
|
||||
fn select_attempt(
|
||||
attempts: &[AttemptDiffData],
|
||||
attempt: Option<usize>,
|
||||
) -> anyhow::Result<&AttemptDiffData> {
|
||||
if attempts.is_empty() {
|
||||
anyhow::bail!("No attempts available");
|
||||
}
|
||||
let desired = attempt.unwrap_or(1);
|
||||
let idx = desired
|
||||
.checked_sub(1)
|
||||
.ok_or_else(|| anyhow!("attempt must be at least 1"))?;
|
||||
if idx >= attempts.len() {
|
||||
anyhow::bail!(
|
||||
"Attempt {desired} not available; only {} attempt(s) found",
|
||||
attempts.len()
|
||||
);
|
||||
}
|
||||
Ok(&attempts[idx])
|
||||
}
|
||||
|
||||
fn task_status_label(status: &TaskStatus) -> &'static str {
|
||||
match status {
|
||||
TaskStatus::Pending => "PENDING",
|
||||
TaskStatus::Ready => "READY",
|
||||
TaskStatus::Applied => "APPLIED",
|
||||
TaskStatus::Error => "ERROR",
|
||||
}
|
||||
}
|
||||
|
||||
fn summary_line(summary: &codex_cloud_tasks_client::DiffSummary, colorize: bool) -> String {
|
||||
if summary.files_changed == 0 && summary.lines_added == 0 && summary.lines_removed == 0 {
|
||||
let base = "no diff";
|
||||
return if colorize {
|
||||
base.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string()
|
||||
} else {
|
||||
base.to_string()
|
||||
};
|
||||
}
|
||||
let adds = summary.lines_added;
|
||||
let dels = summary.lines_removed;
|
||||
let files = summary.files_changed;
|
||||
if colorize {
|
||||
let adds_raw = format!("+{adds}");
|
||||
let adds_str = adds_raw
|
||||
.as_str()
|
||||
.if_supports_color(Stream::Stdout, |t| t.green())
|
||||
.to_string();
|
||||
let dels_raw = format!("-{dels}");
|
||||
let dels_str = dels_raw
|
||||
.as_str()
|
||||
.if_supports_color(Stream::Stdout, |t| t.red())
|
||||
.to_string();
|
||||
let bullet = "•"
|
||||
.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string();
|
||||
let file_label = "file"
|
||||
.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string();
|
||||
let plural = if files == 1 { "" } else { "s" };
|
||||
format!("{adds_str}/{dels_str} {bullet} {files} {file_label}{plural}")
|
||||
} else {
|
||||
format!(
|
||||
"+{adds}/-{dels} • {files} file{}",
|
||||
if files == 1 { "" } else { "s" }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
fn format_task_status_lines(
|
||||
task: &codex_cloud_tasks_client::TaskSummary,
|
||||
now: chrono::DateTime<Utc>,
|
||||
colorize: bool,
|
||||
) -> Vec<String> {
|
||||
let mut lines = Vec::new();
|
||||
let status = task_status_label(&task.status);
|
||||
let status = if colorize {
|
||||
match task.status {
|
||||
TaskStatus::Ready => status
|
||||
.if_supports_color(Stream::Stdout, |t| t.green())
|
||||
.to_string(),
|
||||
TaskStatus::Pending => status
|
||||
.if_supports_color(Stream::Stdout, |t| t.magenta())
|
||||
.to_string(),
|
||||
TaskStatus::Applied => status
|
||||
.if_supports_color(Stream::Stdout, |t| t.blue())
|
||||
.to_string(),
|
||||
TaskStatus::Error => status
|
||||
.if_supports_color(Stream::Stdout, |t| t.red())
|
||||
.to_string(),
|
||||
}
|
||||
} else {
|
||||
status.to_string()
|
||||
};
|
||||
lines.push(format!("[{status}] {}", task.title));
|
||||
let mut meta_parts = Vec::new();
|
||||
if let Some(label) = task.environment_label.as_deref().filter(|s| !s.is_empty()) {
|
||||
if colorize {
|
||||
meta_parts.push(
|
||||
label
|
||||
.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string(),
|
||||
);
|
||||
} else {
|
||||
meta_parts.push(label.to_string());
|
||||
}
|
||||
} else if let Some(id) = task.environment_id.as_deref() {
|
||||
if colorize {
|
||||
meta_parts.push(
|
||||
id.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string(),
|
||||
);
|
||||
} else {
|
||||
meta_parts.push(id.to_string());
|
||||
}
|
||||
}
|
||||
let when = format_relative_time(now, task.updated_at);
|
||||
meta_parts.push(if colorize {
|
||||
when.as_str()
|
||||
.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string()
|
||||
} else {
|
||||
when
|
||||
});
|
||||
let sep = if colorize {
|
||||
" • "
|
||||
.if_supports_color(Stream::Stdout, |t| t.dimmed())
|
||||
.to_string()
|
||||
} else {
|
||||
" • ".to_string()
|
||||
};
|
||||
lines.push(meta_parts.join(&sep));
|
||||
lines.push(summary_line(&task.summary, colorize));
|
||||
lines
|
||||
}
|
||||
|
||||
async fn run_status_command(args: crate::cli::StatusCommand) -> anyhow::Result<()> {
|
||||
let ctx = init_backend("codex_cloud_tasks_status").await?;
|
||||
let task_id = parse_task_id(&args.task_id)?;
|
||||
let summary =
|
||||
codex_cloud_tasks_client::CloudBackend::get_task_summary(&*ctx.backend, task_id).await?;
|
||||
let now = Utc::now();
|
||||
let colorize = supports_color::on(SupportStream::Stdout).is_some();
|
||||
for line in format_task_status_lines(&summary, now, colorize) {
|
||||
println!("{line}");
|
||||
}
|
||||
if !matches!(summary.status, TaskStatus::Ready) {
|
||||
std::process::exit(1);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_diff_command(args: crate::cli::DiffCommand) -> anyhow::Result<()> {
|
||||
let ctx = init_backend("codex_cloud_tasks_diff").await?;
|
||||
let task_id = parse_task_id(&args.task_id)?;
|
||||
let attempts = collect_attempt_diffs(&*ctx.backend, &task_id).await?;
|
||||
let selected = select_attempt(&attempts, args.attempt)?;
|
||||
print!("{}", selected.diff);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_apply_command(args: crate::cli::ApplyCommand) -> anyhow::Result<()> {
|
||||
let ctx = init_backend("codex_cloud_tasks_apply").await?;
|
||||
let task_id = parse_task_id(&args.task_id)?;
|
||||
let attempts = collect_attempt_diffs(&*ctx.backend, &task_id).await?;
|
||||
let selected = select_attempt(&attempts, args.attempt)?;
|
||||
let outcome = codex_cloud_tasks_client::CloudBackend::apply_task(
|
||||
&*ctx.backend,
|
||||
task_id,
|
||||
Some(selected.diff.clone()),
|
||||
)
|
||||
.await?;
|
||||
println!("{}", outcome.message);
|
||||
if !matches!(
|
||||
outcome.status,
|
||||
codex_cloud_tasks_client::ApplyStatus::Success
|
||||
) {
|
||||
std::process::exit(1);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn level_from_status(status: codex_cloud_tasks_client::ApplyStatus) -> app::ApplyResultLevel {
|
||||
match status {
|
||||
codex_cloud_tasks_client::ApplyStatus::Success => app::ApplyResultLevel::Success,
|
||||
@@ -322,6 +596,9 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
|
||||
if let Some(command) = cli.command {
|
||||
return match command {
|
||||
crate::cli::Command::Exec(args) => run_exec_command(args).await,
|
||||
crate::cli::Command::Status(args) => run_status_command(args).await,
|
||||
crate::cli::Command::Apply(args) => run_apply_command(args).await,
|
||||
crate::cli::Command::Diff(args) => run_diff_command(args).await,
|
||||
};
|
||||
}
|
||||
let Cli { .. } = cli;
|
||||
@@ -1713,14 +1990,111 @@ fn pretty_lines_from_error(raw: &str) -> Vec<String> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_cloud_tasks_client::DiffSummary;
|
||||
use codex_cloud_tasks_client::MockClient;
|
||||
use codex_cloud_tasks_client::TaskId;
|
||||
use codex_cloud_tasks_client::TaskStatus;
|
||||
use codex_cloud_tasks_client::TaskSummary;
|
||||
use codex_tui::ComposerAction;
|
||||
use codex_tui::ComposerInput;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
|
||||
#[test]
|
||||
fn format_task_status_lines_with_diff_and_label() {
|
||||
let now = Utc::now();
|
||||
let task = TaskSummary {
|
||||
id: TaskId("task_1".to_string()),
|
||||
title: "Example task".to_string(),
|
||||
status: TaskStatus::Ready,
|
||||
updated_at: now,
|
||||
environment_id: Some("env-1".to_string()),
|
||||
environment_label: Some("Env".to_string()),
|
||||
summary: DiffSummary {
|
||||
files_changed: 3,
|
||||
lines_added: 5,
|
||||
lines_removed: 2,
|
||||
},
|
||||
is_review: false,
|
||||
attempt_total: None,
|
||||
};
|
||||
let lines = format_task_status_lines(&task, now, false);
|
||||
assert_eq!(
|
||||
lines,
|
||||
vec![
|
||||
"[READY] Example task".to_string(),
|
||||
"Env • 0s ago".to_string(),
|
||||
"+5/-2 • 3 files".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_task_status_lines_without_diff_falls_back() {
|
||||
let now = Utc::now();
|
||||
let task = TaskSummary {
|
||||
id: TaskId("task_2".to_string()),
|
||||
title: "No diff task".to_string(),
|
||||
status: TaskStatus::Pending,
|
||||
updated_at: now,
|
||||
environment_id: Some("env-2".to_string()),
|
||||
environment_label: None,
|
||||
summary: DiffSummary::default(),
|
||||
is_review: false,
|
||||
attempt_total: Some(1),
|
||||
};
|
||||
let lines = format_task_status_lines(&task, now, false);
|
||||
assert_eq!(
|
||||
lines,
|
||||
vec![
|
||||
"[PENDING] No diff task".to_string(),
|
||||
"env-2 • 0s ago".to_string(),
|
||||
"no diff".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn collect_attempt_diffs_includes_sibling_attempts() {
|
||||
let backend = MockClient;
|
||||
let task_id = parse_task_id("https://chatgpt.com/codex/tasks/T-1000").expect("id");
|
||||
let attempts = collect_attempt_diffs(&backend, &task_id)
|
||||
.await
|
||||
.expect("attempts");
|
||||
assert_eq!(attempts.len(), 2);
|
||||
assert_eq!(attempts[0].placement, Some(0));
|
||||
assert_eq!(attempts[1].placement, Some(1));
|
||||
assert!(!attempts[0].diff.is_empty());
|
||||
assert!(!attempts[1].diff.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_attempt_validates_bounds() {
|
||||
let attempts = vec![AttemptDiffData {
|
||||
placement: Some(0),
|
||||
created_at: None,
|
||||
diff: "diff --git a/file b/file\n".to_string(),
|
||||
}];
|
||||
let first = select_attempt(&attempts, Some(1)).expect("attempt 1");
|
||||
assert_eq!(first.diff, "diff --git a/file b/file\n");
|
||||
assert!(select_attempt(&attempts, Some(2)).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_task_id_from_url_and_raw() {
|
||||
let raw = parse_task_id("task_i_abc123").expect("raw id");
|
||||
assert_eq!(raw.0, "task_i_abc123");
|
||||
let url =
|
||||
parse_task_id("https://chatgpt.com/codex/tasks/task_i_123456?foo=bar").expect("url id");
|
||||
assert_eq!(url.0, "task_i_123456");
|
||||
assert!(parse_task_id(" ").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "very slow"]
|
||||
fn composer_input_renders_typed_characters() {
|
||||
|
||||
@@ -20,8 +20,7 @@ use std::time::Instant;
|
||||
|
||||
use crate::app::App;
|
||||
use crate::app::AttemptView;
|
||||
use chrono::Local;
|
||||
use chrono::Utc;
|
||||
use crate::util::format_relative_time_now;
|
||||
use codex_cloud_tasks_client::AttemptStatus;
|
||||
use codex_cloud_tasks_client::TaskStatus;
|
||||
use codex_tui::render_markdown_text;
|
||||
@@ -804,7 +803,7 @@ fn render_task_item(_app: &App, t: &codex_cloud_tasks_client::TaskSummary) -> Li
|
||||
if let Some(lbl) = t.environment_label.as_ref().filter(|s| !s.is_empty()) {
|
||||
meta.push(lbl.clone().dim());
|
||||
}
|
||||
let when = format_relative_time(t.updated_at).dim();
|
||||
let when = format_relative_time_now(t.updated_at).dim();
|
||||
if !meta.is_empty() {
|
||||
meta.push(" ".into());
|
||||
meta.push("•".dim());
|
||||
@@ -841,27 +840,6 @@ fn render_task_item(_app: &App, t: &codex_cloud_tasks_client::TaskSummary) -> Li
|
||||
ListItem::new(vec![title, meta_line, sub, spacer])
|
||||
}
|
||||
|
||||
fn format_relative_time(ts: chrono::DateTime<Utc>) -> String {
|
||||
let now = Utc::now();
|
||||
let mut secs = (now - ts).num_seconds();
|
||||
if secs < 0 {
|
||||
secs = 0;
|
||||
}
|
||||
if secs < 60 {
|
||||
return format!("{secs}s ago");
|
||||
}
|
||||
let mins = secs / 60;
|
||||
if mins < 60 {
|
||||
return format!("{mins}m ago");
|
||||
}
|
||||
let hours = mins / 60;
|
||||
if hours < 24 {
|
||||
return format!("{hours}h ago");
|
||||
}
|
||||
let local = ts.with_timezone(&Local);
|
||||
local.format("%b %e %H:%M").to_string()
|
||||
}
|
||||
|
||||
fn draw_inline_spinner(
|
||||
frame: &mut Frame,
|
||||
area: Rect,
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use base64::Engine as _;
|
||||
use chrono::DateTime;
|
||||
use chrono::Local;
|
||||
use chrono::Utc;
|
||||
use reqwest::header::HeaderMap;
|
||||
|
||||
@@ -120,3 +122,27 @@ pub fn task_url(base_url: &str, task_id: &str) -> String {
|
||||
}
|
||||
format!("{normalized}/codex/tasks/{task_id}")
|
||||
}
|
||||
|
||||
pub fn format_relative_time(reference: DateTime<Utc>, ts: DateTime<Utc>) -> String {
|
||||
let mut secs = (reference - ts).num_seconds();
|
||||
if secs < 0 {
|
||||
secs = 0;
|
||||
}
|
||||
if secs < 60 {
|
||||
return format!("{secs}s ago");
|
||||
}
|
||||
let mins = secs / 60;
|
||||
if mins < 60 {
|
||||
return format!("{mins}m ago");
|
||||
}
|
||||
let hours = mins / 60;
|
||||
if hours < 24 {
|
||||
return format!("{hours}h ago");
|
||||
}
|
||||
let local = ts.with_timezone(&Local);
|
||||
local.format("%b %e %H:%M").to_string()
|
||||
}
|
||||
|
||||
pub fn format_relative_time_now(ts: DateTime<Utc>) -> String {
|
||||
format_relative_time(Utc::now(), ts)
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ use eventsource_stream::Eventsource;
|
||||
use futures::Stream;
|
||||
use futures::StreamExt;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::time::Instant;
|
||||
@@ -41,12 +42,17 @@ pub async fn process_chat_sse<S>(
|
||||
|
||||
#[derive(Default, Debug)]
|
||||
struct ToolCallState {
|
||||
id: Option<String>,
|
||||
name: Option<String>,
|
||||
arguments: String,
|
||||
}
|
||||
|
||||
let mut tool_calls: HashMap<String, ToolCallState> = HashMap::new();
|
||||
let mut tool_call_order: Vec<String> = Vec::new();
|
||||
let mut tool_calls: HashMap<usize, ToolCallState> = HashMap::new();
|
||||
let mut tool_call_order: Vec<usize> = Vec::new();
|
||||
let mut tool_call_order_seen: HashSet<usize> = HashSet::new();
|
||||
let mut tool_call_index_by_id: HashMap<String, usize> = HashMap::new();
|
||||
let mut next_tool_call_index = 0usize;
|
||||
let mut last_tool_call_index: Option<usize> = None;
|
||||
let mut assistant_item: Option<ResponseItem> = None;
|
||||
let mut reasoning_item: Option<ResponseItem> = None;
|
||||
let mut completed_sent = false;
|
||||
@@ -149,15 +155,40 @@ pub async fn process_chat_sse<S>(
|
||||
|
||||
if let Some(tool_call_values) = delta.get("tool_calls").and_then(|c| c.as_array()) {
|
||||
for tool_call in tool_call_values {
|
||||
let id = tool_call
|
||||
.get("id")
|
||||
.and_then(|i| i.as_str())
|
||||
.map(str::to_string)
|
||||
.unwrap_or_else(|| format!("tool-call-{}", tool_call_order.len()));
|
||||
let mut index = tool_call
|
||||
.get("index")
|
||||
.and_then(serde_json::Value::as_u64)
|
||||
.map(|i| i as usize);
|
||||
|
||||
let call_state = tool_calls.entry(id.clone()).or_default();
|
||||
if !tool_call_order.contains(&id) {
|
||||
tool_call_order.push(id.clone());
|
||||
let mut call_id_for_lookup = None;
|
||||
if let Some(call_id) = tool_call.get("id").and_then(|i| i.as_str()) {
|
||||
call_id_for_lookup = Some(call_id.to_string());
|
||||
if let Some(existing) = tool_call_index_by_id.get(call_id) {
|
||||
index = Some(*existing);
|
||||
}
|
||||
}
|
||||
|
||||
if index.is_none() && call_id_for_lookup.is_none() {
|
||||
index = last_tool_call_index;
|
||||
}
|
||||
|
||||
let index = index.unwrap_or_else(|| {
|
||||
while tool_calls.contains_key(&next_tool_call_index) {
|
||||
next_tool_call_index += 1;
|
||||
}
|
||||
let idx = next_tool_call_index;
|
||||
next_tool_call_index += 1;
|
||||
idx
|
||||
});
|
||||
|
||||
let call_state = tool_calls.entry(index).or_default();
|
||||
if tool_call_order_seen.insert(index) {
|
||||
tool_call_order.push(index);
|
||||
}
|
||||
|
||||
if let Some(id) = tool_call.get("id").and_then(|i| i.as_str()) {
|
||||
call_state.id.get_or_insert_with(|| id.to_string());
|
||||
tool_call_index_by_id.entry(id.to_string()).or_insert(index);
|
||||
}
|
||||
|
||||
if let Some(func) = tool_call.get("function") {
|
||||
@@ -171,6 +202,8 @@ pub async fn process_chat_sse<S>(
|
||||
call_state.arguments.push_str(arguments);
|
||||
}
|
||||
}
|
||||
|
||||
last_tool_call_index = Some(index);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -224,13 +257,25 @@ pub async fn process_chat_sse<S>(
|
||||
.await;
|
||||
}
|
||||
|
||||
for call_id in tool_call_order.drain(..) {
|
||||
let state = tool_calls.remove(&call_id).unwrap_or_default();
|
||||
for index in tool_call_order.drain(..) {
|
||||
let Some(state) = tool_calls.remove(&index) else {
|
||||
continue;
|
||||
};
|
||||
tool_call_order_seen.remove(&index);
|
||||
let ToolCallState {
|
||||
id,
|
||||
name,
|
||||
arguments,
|
||||
} = state;
|
||||
let Some(name) = name else {
|
||||
debug!("Skipping tool call at index {index} because name is missing");
|
||||
continue;
|
||||
};
|
||||
let item = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: state.name.unwrap_or_default(),
|
||||
arguments: state.arguments,
|
||||
call_id: call_id.clone(),
|
||||
name,
|
||||
arguments,
|
||||
call_id: id.unwrap_or_else(|| format!("tool-call-{index}")),
|
||||
};
|
||||
let _ = tx_event.send(Ok(ResponseEvent::OutputItemDone(item))).await;
|
||||
}
|
||||
@@ -335,6 +380,59 @@ mod tests {
|
||||
out
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn concatenates_tool_call_arguments_across_deltas() {
|
||||
let delta_name = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"id": "call_a",
|
||||
"index": 0,
|
||||
"function": { "name": "do_a" }
|
||||
}]
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
let delta_args_1 = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"index": 0,
|
||||
"function": { "arguments": "{ \"foo\":" }
|
||||
}]
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
let delta_args_2 = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"index": 0,
|
||||
"function": { "arguments": "1}" }
|
||||
}]
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
let finish = json!({
|
||||
"choices": [{
|
||||
"finish_reason": "tool_calls"
|
||||
}]
|
||||
});
|
||||
|
||||
let body = build_body(&[delta_name, delta_args_1, delta_args_2, finish]);
|
||||
let events = collect_events(&body).await;
|
||||
assert_matches!(
|
||||
&events[..],
|
||||
[
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id, name, arguments, .. }),
|
||||
ResponseEvent::Completed { .. }
|
||||
] if call_id == "call_a" && name == "do_a" && arguments == "{ \"foo\":1}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emits_multiple_tool_calls() {
|
||||
let delta_a = json!({
|
||||
@@ -367,50 +465,74 @@ mod tests {
|
||||
|
||||
let body = build_body(&[delta_a, delta_b, finish]);
|
||||
let events = collect_events(&body).await;
|
||||
assert_eq!(events.len(), 3);
|
||||
|
||||
assert_matches!(
|
||||
&events[0],
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id, name, arguments, .. })
|
||||
if call_id == "call_a" && name == "do_a" && arguments == "{\"foo\":1}"
|
||||
&events[..],
|
||||
[
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id: call_a, name: name_a, arguments: args_a, .. }),
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id: call_b, name: name_b, arguments: args_b, .. }),
|
||||
ResponseEvent::Completed { .. }
|
||||
] if call_a == "call_a" && name_a == "do_a" && args_a == "{\"foo\":1}" && call_b == "call_b" && name_b == "do_b" && args_b == "{\"bar\":2}"
|
||||
);
|
||||
assert_matches!(
|
||||
&events[1],
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id, name, arguments, .. })
|
||||
if call_id == "call_b" && name == "do_b" && arguments == "{\"bar\":2}"
|
||||
);
|
||||
assert_matches!(events[2], ResponseEvent::Completed { .. });
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn concatenates_tool_call_arguments_across_deltas() {
|
||||
let delta_name = json!({
|
||||
async fn emits_tool_calls_for_multiple_choices() {
|
||||
let payload = json!({
|
||||
"choices": [
|
||||
{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"id": "call_a",
|
||||
"index": 0,
|
||||
"function": { "name": "do_a", "arguments": "{}" }
|
||||
}]
|
||||
},
|
||||
"finish_reason": "tool_calls"
|
||||
},
|
||||
{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"id": "call_b",
|
||||
"index": 0,
|
||||
"function": { "name": "do_b", "arguments": "{}" }
|
||||
}]
|
||||
},
|
||||
"finish_reason": "tool_calls"
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
let body = build_body(&[payload]);
|
||||
let events = collect_events(&body).await;
|
||||
assert_matches!(
|
||||
&events[..],
|
||||
[
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id: call_a, name: name_a, arguments: args_a, .. }),
|
||||
ResponseEvent::OutputItemDone(ResponseItem::FunctionCall { call_id: call_b, name: name_b, arguments: args_b, .. }),
|
||||
ResponseEvent::Completed { .. }
|
||||
] if call_a == "call_a" && name_a == "do_a" && args_a == "{}" && call_b == "call_b" && name_b == "do_b" && args_b == "{}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn merges_tool_calls_by_index_when_id_missing_on_subsequent_deltas() {
|
||||
let delta_with_id = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"index": 0,
|
||||
"id": "call_a",
|
||||
"function": { "name": "do_a" }
|
||||
"function": { "name": "do_a", "arguments": "{ \"foo\":" }
|
||||
}]
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
let delta_args_1 = json!({
|
||||
let delta_without_id = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"id": "call_a",
|
||||
"function": { "arguments": "{ \"foo\":" }
|
||||
}]
|
||||
}
|
||||
}]
|
||||
});
|
||||
|
||||
let delta_args_2 = json!({
|
||||
"choices": [{
|
||||
"delta": {
|
||||
"tool_calls": [{
|
||||
"id": "call_a",
|
||||
"index": 0,
|
||||
"function": { "arguments": "1}" }
|
||||
}]
|
||||
}
|
||||
@@ -423,7 +545,7 @@ mod tests {
|
||||
}]
|
||||
});
|
||||
|
||||
let body = build_body(&[delta_name, delta_args_1, delta_args_2, finish]);
|
||||
let body = build_body(&[delta_with_id, delta_without_id, finish]);
|
||||
let events = collect_events(&body).await;
|
||||
assert_matches!(
|
||||
&events[..],
|
||||
|
||||
@@ -90,6 +90,7 @@ wildmatch = { workspace = true }
|
||||
|
||||
[features]
|
||||
deterministic_process_ids = []
|
||||
test-support = []
|
||||
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
|
||||
+55
-37
@@ -14,6 +14,7 @@ use crate::compact_remote::run_inline_remote_auto_compact_task;
|
||||
use crate::exec_policy::load_exec_policy_for_features;
|
||||
use crate::features::Feature;
|
||||
use crate::features::Features;
|
||||
use crate::openai_models::model_family::ModelFamily;
|
||||
use crate::openai_models::models_manager::ModelsManager;
|
||||
use crate::parse_command::parse_command;
|
||||
use crate::parse_turn_item;
|
||||
@@ -398,35 +399,39 @@ pub(crate) struct SessionSettingsUpdate {
|
||||
}
|
||||
|
||||
impl Session {
|
||||
fn make_turn_context(
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
models_manager: Arc<ModelsManager>,
|
||||
otel_event_manager: &OtelEventManager,
|
||||
provider: ModelProviderInfo,
|
||||
session_configuration: &SessionConfiguration,
|
||||
conversation_id: ConversationId,
|
||||
sub_id: String,
|
||||
) -> TurnContext {
|
||||
fn build_per_turn_config(session_configuration: &SessionConfiguration) -> Config {
|
||||
let config = session_configuration.original_config_do_not_use.clone();
|
||||
let features = &config.features;
|
||||
let mut per_turn_config = (*config).clone();
|
||||
per_turn_config.model = session_configuration.model.clone();
|
||||
per_turn_config.model_reasoning_effort = session_configuration.model_reasoning_effort;
|
||||
per_turn_config.model_reasoning_summary = session_configuration.model_reasoning_summary;
|
||||
per_turn_config.features = features.clone();
|
||||
let model_family =
|
||||
models_manager.construct_model_family(&per_turn_config.model, &per_turn_config);
|
||||
per_turn_config.features = config.features.clone();
|
||||
per_turn_config
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn make_turn_context(
|
||||
auth_manager: Option<Arc<AuthManager>>,
|
||||
otel_event_manager: &OtelEventManager,
|
||||
provider: ModelProviderInfo,
|
||||
session_configuration: &SessionConfiguration,
|
||||
mut per_turn_config: Config,
|
||||
model_family: ModelFamily,
|
||||
conversation_id: ConversationId,
|
||||
sub_id: String,
|
||||
) -> TurnContext {
|
||||
if let Some(model_info) = get_model_info(&model_family) {
|
||||
per_turn_config.model_context_window = Some(model_info.context_window);
|
||||
}
|
||||
|
||||
let otel_event_manager = otel_event_manager.clone().with_model(
|
||||
session_configuration.model.as_str(),
|
||||
session_configuration.model.as_str(),
|
||||
model_family.slug.as_str(),
|
||||
);
|
||||
|
||||
let per_turn_config = Arc::new(per_turn_config);
|
||||
let client = ModelClient::new(
|
||||
Arc::new(per_turn_config.clone()),
|
||||
per_turn_config.clone(),
|
||||
auth_manager,
|
||||
model_family.clone(),
|
||||
otel_event_manager,
|
||||
@@ -439,7 +444,7 @@ impl Session {
|
||||
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_family: &model_family,
|
||||
features,
|
||||
features: &per_turn_config.features,
|
||||
});
|
||||
|
||||
TurnContext {
|
||||
@@ -452,14 +457,14 @@ impl Session {
|
||||
user_instructions: session_configuration.user_instructions.clone(),
|
||||
approval_policy: session_configuration.approval_policy,
|
||||
sandbox_policy: session_configuration.sandbox_policy.clone(),
|
||||
shell_environment_policy: config.shell_environment_policy.clone(),
|
||||
shell_environment_policy: per_turn_config.shell_environment_policy.clone(),
|
||||
tools_config,
|
||||
final_output_json_schema: None,
|
||||
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
|
||||
codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
exec_policy: session_configuration.exec_policy.clone(),
|
||||
truncation_policy: TruncationPolicy::new(
|
||||
&per_turn_config,
|
||||
per_turn_config.as_ref(),
|
||||
model_family.truncation_policy,
|
||||
),
|
||||
}
|
||||
@@ -545,7 +550,9 @@ impl Session {
|
||||
});
|
||||
}
|
||||
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = models_manager
|
||||
.construct_model_family(&config.model, &config)
|
||||
.await;
|
||||
// todo(aibrahim): why are we passing model here while it can change?
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
@@ -768,12 +775,19 @@ impl Session {
|
||||
session_configuration
|
||||
};
|
||||
|
||||
let per_turn_config = Self::build_per_turn_config(&session_configuration);
|
||||
let model_family = self
|
||||
.services
|
||||
.models_manager
|
||||
.construct_model_family(&per_turn_config.model, &per_turn_config)
|
||||
.await;
|
||||
let mut turn_context: TurnContext = Self::make_turn_context(
|
||||
Some(Arc::clone(&self.services.auth_manager)),
|
||||
Arc::clone(&self.services.models_manager),
|
||||
&self.services.otel_event_manager,
|
||||
session_configuration.provider.clone(),
|
||||
&session_configuration,
|
||||
per_turn_config,
|
||||
model_family,
|
||||
self.conversation_id,
|
||||
sub_id,
|
||||
);
|
||||
@@ -1907,7 +1921,8 @@ async fn spawn_review_thread(
|
||||
let review_model_family = sess
|
||||
.services
|
||||
.models_manager
|
||||
.construct_model_family(&model, &config);
|
||||
.construct_model_family(&model, &config)
|
||||
.await;
|
||||
// For reviews, disable web_search and view_image regardless of global settings.
|
||||
let mut review_features = sess.features.clone();
|
||||
review_features
|
||||
@@ -2812,15 +2827,12 @@ mod tests {
|
||||
fn otel_event_manager(
|
||||
conversation_id: ConversationId,
|
||||
config: &Config,
|
||||
models_manager: &ModelsManager,
|
||||
model_family: &ModelFamily,
|
||||
) -> OtelEventManager {
|
||||
OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
models_manager
|
||||
.construct_model_family(&config.model, config)
|
||||
.slug
|
||||
.as_str(),
|
||||
model_family.slug.as_str(),
|
||||
None,
|
||||
Some("test@test.com".to_string()),
|
||||
Some(AuthMode::ChatGPT),
|
||||
@@ -2843,9 +2855,6 @@ mod tests {
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
|
||||
let otel_event_manager =
|
||||
otel_event_manager(conversation_id, config.as_ref(), &models_manager);
|
||||
|
||||
let session_configuration = SessionConfiguration {
|
||||
provider: config.model_provider.clone(),
|
||||
model: config.model.clone(),
|
||||
@@ -2862,6 +2871,11 @@ mod tests {
|
||||
exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())),
|
||||
session_source: SessionSource::Exec,
|
||||
};
|
||||
let per_turn_config = Session::build_per_turn_config(&session_configuration);
|
||||
let model_family =
|
||||
ModelsManager::construct_model_family_offline(&per_turn_config.model, &per_turn_config);
|
||||
let otel_event_manager =
|
||||
otel_event_manager(conversation_id, config.as_ref(), &model_family);
|
||||
|
||||
let state = SessionState::new(session_configuration.clone());
|
||||
|
||||
@@ -2875,16 +2889,17 @@ mod tests {
|
||||
show_raw_agent_reasoning: config.show_raw_agent_reasoning,
|
||||
auth_manager: auth_manager.clone(),
|
||||
otel_event_manager: otel_event_manager.clone(),
|
||||
models_manager: models_manager.clone(),
|
||||
models_manager,
|
||||
tool_approvals: Mutex::new(ApprovalStore::default()),
|
||||
};
|
||||
|
||||
let turn_context = Session::make_turn_context(
|
||||
Some(Arc::clone(&auth_manager)),
|
||||
models_manager,
|
||||
&otel_event_manager,
|
||||
session_configuration.provider.clone(),
|
||||
&session_configuration,
|
||||
per_turn_config,
|
||||
model_family,
|
||||
conversation_id,
|
||||
"turn_id".to_string(),
|
||||
);
|
||||
@@ -2922,9 +2937,6 @@ mod tests {
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
|
||||
let otel_event_manager =
|
||||
otel_event_manager(conversation_id, config.as_ref(), &models_manager);
|
||||
|
||||
let session_configuration = SessionConfiguration {
|
||||
provider: config.model_provider.clone(),
|
||||
model: config.model.clone(),
|
||||
@@ -2941,6 +2953,11 @@ mod tests {
|
||||
exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())),
|
||||
session_source: SessionSource::Exec,
|
||||
};
|
||||
let per_turn_config = Session::build_per_turn_config(&session_configuration);
|
||||
let model_family =
|
||||
ModelsManager::construct_model_family_offline(&per_turn_config.model, &per_turn_config);
|
||||
let otel_event_manager =
|
||||
otel_event_manager(conversation_id, config.as_ref(), &model_family);
|
||||
|
||||
let state = SessionState::new(session_configuration.clone());
|
||||
|
||||
@@ -2954,16 +2971,17 @@ mod tests {
|
||||
show_raw_agent_reasoning: config.show_raw_agent_reasoning,
|
||||
auth_manager: Arc::clone(&auth_manager),
|
||||
otel_event_manager: otel_event_manager.clone(),
|
||||
models_manager: models_manager.clone(),
|
||||
models_manager,
|
||||
tool_approvals: Mutex::new(ApprovalStore::default()),
|
||||
};
|
||||
|
||||
let turn_context = Arc::new(Session::make_turn_context(
|
||||
Some(Arc::clone(&auth_manager)),
|
||||
models_manager,
|
||||
&otel_event_manager,
|
||||
session_configuration.provider.clone(),
|
||||
&session_configuration,
|
||||
per_turn_config,
|
||||
model_family,
|
||||
conversation_id,
|
||||
"turn_id".to_string(),
|
||||
));
|
||||
|
||||
@@ -61,7 +61,14 @@ impl ModelsManager {
|
||||
Ok(models)
|
||||
}
|
||||
|
||||
pub fn construct_model_family(&self, model: &str, config: &Config) -> ModelFamily {
|
||||
pub async fn construct_model_family(&self, model: &str, config: &Config) -> ModelFamily {
|
||||
find_family_for_model(model)
|
||||
.with_config_overrides(config)
|
||||
.with_remote_overrides(self.remote_models.read().await.clone())
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub fn construct_model_family_offline(model: &str, config: &Config) -> ModelFamily {
|
||||
find_family_for_model(model).with_config_overrides(config)
|
||||
}
|
||||
|
||||
|
||||
@@ -126,7 +126,9 @@ pub(crate) async fn assess_command(
|
||||
output_schema: Some(sandbox_assessment_schema()),
|
||||
};
|
||||
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = models_manager
|
||||
.construct_model_family(&config.model, &config)
|
||||
.await;
|
||||
|
||||
let child_otel = parent_otel.with_model(config.model.as_str(), model_family.slug.as_str());
|
||||
|
||||
|
||||
@@ -408,6 +408,48 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_exec_args() {
|
||||
let test_bash_shell = Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path: PathBuf::from("/bin/bash"),
|
||||
};
|
||||
assert_eq!(
|
||||
test_bash_shell.derive_exec_args("echo hello", false),
|
||||
vec!["/bin/bash", "-c", "echo hello"]
|
||||
);
|
||||
assert_eq!(
|
||||
test_bash_shell.derive_exec_args("echo hello", true),
|
||||
vec!["/bin/bash", "-lc", "echo hello"]
|
||||
);
|
||||
|
||||
let test_zsh_shell = Shell {
|
||||
shell_type: ShellType::Zsh,
|
||||
shell_path: PathBuf::from("/bin/zsh"),
|
||||
};
|
||||
assert_eq!(
|
||||
test_zsh_shell.derive_exec_args("echo hello", false),
|
||||
vec!["/bin/zsh", "-c", "echo hello"]
|
||||
);
|
||||
assert_eq!(
|
||||
test_zsh_shell.derive_exec_args("echo hello", true),
|
||||
vec!["/bin/zsh", "-lc", "echo hello"]
|
||||
);
|
||||
|
||||
let test_powershell_shell = Shell {
|
||||
shell_type: ShellType::PowerShell,
|
||||
shell_path: PathBuf::from("pwsh.exe"),
|
||||
};
|
||||
assert_eq!(
|
||||
test_powershell_shell.derive_exec_args("echo hello", false),
|
||||
vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"]
|
||||
);
|
||||
assert_eq!(
|
||||
test_powershell_shell.derive_exec_args("echo hello", true),
|
||||
vec!["pwsh.exe", "-Command", "echo hello"]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_current_shell_detects_zsh() {
|
||||
let shell = Command::new("sh")
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::config::Config;
|
||||
use crate::git_info::resolve_root_git_project_for_trust;
|
||||
use crate::skills::model::SkillError;
|
||||
use crate::skills::model::SkillLoadOutcome;
|
||||
use crate::skills::model::SkillMetadata;
|
||||
@@ -20,6 +21,7 @@ struct SkillFrontmatter {
|
||||
|
||||
const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
const SKILLS_DIR_NAME: &str = "skills";
|
||||
const REPO_ROOT_CONFIG_DIR_NAME: &str = ".codex";
|
||||
const MAX_NAME_LEN: usize = 100;
|
||||
const MAX_DESCRIPTION_LEN: usize = 500;
|
||||
|
||||
@@ -65,7 +67,17 @@ pub fn load_skills(config: &Config) -> SkillLoadOutcome {
|
||||
}
|
||||
|
||||
fn skill_roots(config: &Config) -> Vec<PathBuf> {
|
||||
vec![config.codex_home.join(SKILLS_DIR_NAME)]
|
||||
let mut roots = vec![config.codex_home.join(SKILLS_DIR_NAME)];
|
||||
|
||||
if let Some(repo_root) = resolve_root_git_project_for_trust(&config.cwd) {
|
||||
roots.push(
|
||||
repo_root
|
||||
.join(REPO_ROOT_CONFIG_DIR_NAME)
|
||||
.join(SKILLS_DIR_NAME),
|
||||
);
|
||||
}
|
||||
|
||||
roots
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, outcome: &mut SkillLoadOutcome) {
|
||||
@@ -196,6 +208,9 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn make_config(codex_home: &TempDir) -> Config {
|
||||
@@ -211,7 +226,11 @@ mod tests {
|
||||
}
|
||||
|
||||
fn write_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) -> PathBuf {
|
||||
let skill_dir = codex_home.path().join(format!("skills/{dir}"));
|
||||
write_skill_at(codex_home.path(), dir, name, description)
|
||||
}
|
||||
|
||||
fn write_skill_at(root: &Path, dir: &str, name: &str, description: &str) -> PathBuf {
|
||||
let skill_dir = root.join(format!("skills/{dir}"));
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
let indented_description = description.replace('\n', "\n ");
|
||||
let content = format!(
|
||||
@@ -288,4 +307,37 @@ mod tests {
|
||||
"expected length error"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loads_skills_from_repo_root() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let repo_dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let status = Command::new("git")
|
||||
.arg("init")
|
||||
.current_dir(repo_dir.path())
|
||||
.status()
|
||||
.expect("git init");
|
||||
assert!(status.success(), "git init failed");
|
||||
|
||||
let skills_root = repo_dir
|
||||
.path()
|
||||
.join(REPO_ROOT_CONFIG_DIR_NAME)
|
||||
.join(SKILLS_DIR_NAME);
|
||||
write_skill_at(&skills_root, "repo", "repo-skill", "from repo");
|
||||
let mut cfg = make_config(&codex_home);
|
||||
cfg.cwd = repo_dir.path().to_path_buf();
|
||||
let repo_root = normalize_path(&skills_root).unwrap_or_else(|_| skills_root.clone());
|
||||
|
||||
let outcome = load_skills(&cfg);
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
let skill = &outcome.skills[0];
|
||||
assert_eq!(skill.name, "repo-skill");
|
||||
assert!(skill.path.starts_with(&repo_root));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,8 +49,7 @@ impl ShellCommandHandler {
|
||||
turn_context: &TurnContext,
|
||||
) -> ExecParams {
|
||||
let shell = session.user_shell();
|
||||
let use_login_shell = true;
|
||||
let command = shell.derive_exec_args(¶ms.command, use_login_shell);
|
||||
let command = shell.derive_exec_args(¶ms.command, params.login.unwrap_or(true));
|
||||
|
||||
ExecParams {
|
||||
command,
|
||||
@@ -276,9 +275,15 @@ impl ShellHandler {
|
||||
mod tests {
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::models::ShellCommandToolCallParams;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use crate::codex::make_session_and_context;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::handlers::ShellCommandHandler;
|
||||
|
||||
/// The logic for is_known_safe_command() has heuristics for known shells,
|
||||
/// so we must ensure the commands generated by [ShellCommandHandler] can be
|
||||
@@ -312,4 +317,43 @@ mod tests {
|
||||
&shell.derive_exec_args(command, /* use_login_shell */ false)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_context() {
|
||||
let (session, turn_context) = make_session_and_context();
|
||||
|
||||
let command = "echo hello".to_string();
|
||||
let workdir = Some("subdir".to_string());
|
||||
let login = None;
|
||||
let timeout_ms = Some(1234);
|
||||
let with_escalated_permissions = Some(true);
|
||||
let justification = Some("because tests".to_string());
|
||||
|
||||
let expected_command = session.user_shell().derive_exec_args(&command, true);
|
||||
let expected_cwd = turn_context.resolve_path(workdir.clone());
|
||||
let expected_env = create_env(&turn_context.shell_environment_policy);
|
||||
|
||||
let params = ShellCommandToolCallParams {
|
||||
command,
|
||||
workdir,
|
||||
login,
|
||||
timeout_ms,
|
||||
with_escalated_permissions,
|
||||
justification: justification.clone(),
|
||||
};
|
||||
|
||||
let exec_params = ShellCommandHandler::to_exec_params(params, &session, &turn_context);
|
||||
|
||||
// ExecParams cannot derive Eq due to the CancellationToken field, so we manually compare the fields.
|
||||
assert_eq!(exec_params.command, expected_command);
|
||||
assert_eq!(exec_params.cwd, expected_cwd);
|
||||
assert_eq!(exec_params.env, expected_env);
|
||||
assert_eq!(exec_params.expiration.timeout_ms(), timeout_ms);
|
||||
assert_eq!(
|
||||
exec_params.with_escalated_permissions,
|
||||
with_escalated_permissions
|
||||
);
|
||||
assert_eq!(exec_params.justification, justification);
|
||||
assert_eq!(exec_params.arg0, None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -331,6 +331,15 @@ fn create_shell_command_tool() -> ToolSpec {
|
||||
description: Some("The working directory to execute the command in".to_string()),
|
||||
},
|
||||
);
|
||||
properties.insert(
|
||||
"login".to_string(),
|
||||
JsonSchema::Boolean {
|
||||
description: Some(
|
||||
"Whether to run the shell with login shell semantics. Defaults to true."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
);
|
||||
properties.insert(
|
||||
"timeout_ms".to_string(),
|
||||
JsonSchema::Number {
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_core::AuthManager;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::ContentItem;
|
||||
use codex_core::LocalShellAction;
|
||||
use codex_core::LocalShellExecAction;
|
||||
@@ -73,9 +71,7 @@ async fn run_request(input: Vec<ResponseItem>) -> Value {
|
||||
let config = Arc::new(config);
|
||||
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use assert_matches::assert_matches;
|
||||
use codex_core::AuthManager;
|
||||
use codex_core::openai_models::models_manager::ModelsManager;
|
||||
use std::sync::Arc;
|
||||
use tracing_test::traced_test;
|
||||
|
||||
@@ -12,6 +11,7 @@ use codex_core::Prompt;
|
||||
use codex_core::ResponseEvent;
|
||||
use codex_core::ResponseItem;
|
||||
use codex_core::WireApi;
|
||||
use codex_core::openai_models::models_manager::ModelsManager;
|
||||
use codex_otel::otel_event_manager::OtelEventManager;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::models::ReasoningItemContent;
|
||||
@@ -74,8 +74,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec<ResponseEvent> {
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let auth_mode = auth_manager.get_auth_mode();
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
|
||||
@@ -11,7 +11,7 @@ path = "lib.rs"
|
||||
anyhow = { workspace = true }
|
||||
assert_cmd = { workspace = true }
|
||||
base64 = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
codex-core = { workspace = true, features = ["test-support"] }
|
||||
codex-protocol = { workspace = true }
|
||||
notify = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
|
||||
@@ -369,3 +369,13 @@ macro_rules! skip_if_no_network {
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
#[macro_export]
|
||||
macro_rules! skip_if_windows {
|
||||
($return_value:expr $(,)?) => {{
|
||||
if cfg!(target_os = "windows") {
|
||||
println!("Skipping test because it cannot execute on Windows.");
|
||||
return $return_value;
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
@@ -65,9 +65,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_mode = AuthMode::ChatGPT;
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
@@ -157,9 +155,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_mode = AuthMode::ChatGPT;
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
@@ -250,16 +246,16 @@ async fn responses_respects_model_family_overrides_from_config() {
|
||||
let config = Arc::new(config);
|
||||
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let auth_mode =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")).get_auth_mode();
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
model_family.slug.as_str(),
|
||||
None,
|
||||
Some("test@test.com".to_string()),
|
||||
auth_manager.get_auth_mode(),
|
||||
auth_mode,
|
||||
false,
|
||||
"test".to_string(),
|
||||
);
|
||||
|
||||
@@ -1250,3 +1250,94 @@ async fn apply_patch_change_context_disambiguates_target(
|
||||
assert_eq!(contents, "fn a\nx=10\ny=2\nfn b\nx=11\ny=20\n");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Ensure that applying a patch can update a CRLF file with unicode characters.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_updates_unicode_characters(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness().await?;
|
||||
|
||||
let target = harness.path("unicode.txt");
|
||||
fs::write(&target, "first ⚠️\nsecond ❌\nthird 🔥\n")?;
|
||||
|
||||
let patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Update File: {}
|
||||
@@
|
||||
first ⚠️
|
||||
-second ❌
|
||||
+SECOND ✅
|
||||
@@
|
||||
third 🔥
|
||||
+FOURTH
|
||||
*** End of File
|
||||
*** End Patch"#,
|
||||
target.display()
|
||||
);
|
||||
let call_id = "apply-unicode-update";
|
||||
mount_apply_patch(&harness, call_id, patch.as_str(), "ok", model_output).await;
|
||||
|
||||
harness
|
||||
.submit("update unicode characters via apply_patch CLI")
|
||||
.await?;
|
||||
|
||||
let file_contents = fs::read(&target)?;
|
||||
let content = String::from_utf8_lossy(&file_contents);
|
||||
assert_eq!(content, "first ⚠️\nSECOND ✅\nthird 🔥\nFOURTH\n");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Ensure that applying a patch via the CLI preserves CRLF line endings for
|
||||
/// Windows-style inputs even when updating the file contents.
|
||||
#[cfg(windows)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[test_case(ApplyPatchModelOutput::Freeform)]
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_updates_crlf_file_preserves_line_endings(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness().await?;
|
||||
|
||||
let target = harness.path("crlf.txt");
|
||||
fs::write(&target, b"first\r\nsecond\r\nthird\r\n")?;
|
||||
|
||||
let patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Update File: {}
|
||||
@@
|
||||
first
|
||||
-second
|
||||
+SECOND
|
||||
@@
|
||||
third
|
||||
+FOURTH
|
||||
*** End of File
|
||||
*** End Patch"#,
|
||||
target.display()
|
||||
);
|
||||
let call_id = "apply-crlf-update";
|
||||
mount_apply_patch(&harness, call_id, patch.as_str(), "ok", model_output).await;
|
||||
|
||||
harness
|
||||
.submit("update crlf file via apply_patch CLI")
|
||||
.await?;
|
||||
|
||||
let file_contents = fs::read(&target)?;
|
||||
let content = String::from_utf8_lossy(&file_contents);
|
||||
assert!(content.contains("\r\n"));
|
||||
assert_eq!(content, "first\r\nSECOND\r\nthird\r\nFOURTH\r\n");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1015,11 +1015,9 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
|
||||
let effort = config.model_reasoning_effort;
|
||||
let summary = config.model_reasoning_summary;
|
||||
let config = Arc::new(config);
|
||||
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
let conversation_id = ConversationId::new();
|
||||
let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager.clone()));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
|
||||
@@ -46,6 +46,7 @@ mod review;
|
||||
mod rmcp_client;
|
||||
mod rollout_list_find;
|
||||
mod seatbelt;
|
||||
mod shell_command;
|
||||
mod shell_serialization;
|
||||
mod stream_error_allows_next_turn;
|
||||
mod stream_no_completed;
|
||||
|
||||
@@ -137,6 +137,7 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> {
|
||||
let base_instructions = conversation_manager
|
||||
.get_models_manager()
|
||||
.construct_model_family(&config.model, &config)
|
||||
.await
|
||||
.base_instructions
|
||||
.clone();
|
||||
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
use anyhow::Result;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_windows;
|
||||
use core_test_support::test_codex::TestCodexBuilder;
|
||||
use core_test_support::test_codex::TestCodexHarness;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use serde_json::json;
|
||||
|
||||
fn shell_responses(call_id: &str, command: &str, login: Option<bool>) -> Vec<String> {
|
||||
let args = json!({
|
||||
"command": command,
|
||||
"timeout_ms": 2_000,
|
||||
"login": login,
|
||||
});
|
||||
|
||||
#[allow(clippy::expect_used)]
|
||||
let arguments = serde_json::to_string(&args).expect("serialize shell command arguments");
|
||||
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell_command", &arguments),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
]
|
||||
}
|
||||
|
||||
async fn shell_command_harness_with(
|
||||
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
|
||||
) -> Result<TestCodexHarness> {
|
||||
let builder = configure(test_codex()).with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
});
|
||||
TestCodexHarness::with_builder(builder).await
|
||||
}
|
||||
|
||||
async fn mount_shell_responses(
|
||||
harness: &TestCodexHarness,
|
||||
call_id: &str,
|
||||
command: &str,
|
||||
login: Option<bool>,
|
||||
) {
|
||||
mount_sse_sequence(harness.server(), shell_responses(call_id, command, login)).await;
|
||||
}
|
||||
|
||||
fn assert_shell_command_output(output: &str, expected: &str) -> Result<()> {
|
||||
let normalized_output = output
|
||||
.replace("\r\n", "\n")
|
||||
.replace('\r', "\n")
|
||||
.trim_end_matches('\n')
|
||||
.to_string();
|
||||
|
||||
let expected_pattern = format!(
|
||||
r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\n{expected}\n?$"
|
||||
);
|
||||
|
||||
assert_regex_match(&expected_pattern, &normalized_output);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shell_command_works() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call";
|
||||
mount_shell_responses(&harness, call_id, "echo 'hello, world'", None).await;
|
||||
harness.submit("run the echo command").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "hello, world")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn output_with_login() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call-login-true";
|
||||
mount_shell_responses(&harness, call_id, "echo 'hello, world'", Some(true)).await;
|
||||
harness.submit("run the echo command with login").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "hello, world")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn output_without_login() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call-login-false";
|
||||
mount_shell_responses(&harness, call_id, "echo 'hello, world'", Some(false)).await;
|
||||
harness.submit("run the echo command without login").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "hello, world")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn multi_line_output_with_login() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call-first-extra-login";
|
||||
mount_shell_responses(
|
||||
&harness,
|
||||
call_id,
|
||||
"echo 'first line\nsecond line'",
|
||||
Some(true),
|
||||
)
|
||||
.await;
|
||||
harness.submit("run the command with login").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "first line\nsecond line")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn pipe_output_with_login() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call-second-extra-no-login";
|
||||
mount_shell_responses(&harness, call_id, "echo 'hello, world' | cat", None).await;
|
||||
harness.submit("run the command without login").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "hello, world")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn pipe_output_without_login() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
|
||||
let call_id = "shell-command-call-third-extra-login-false";
|
||||
mount_shell_responses(&harness, call_id, "echo 'hello, world' | cat", Some(false)).await;
|
||||
harness.submit("run the command without login").await?;
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert_shell_command_output(&output, "hello, world")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1,4 +1,3 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsStr;
|
||||
use std::fs;
|
||||
@@ -24,6 +23,7 @@ use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_sandbox;
|
||||
use core_test_support::skip_if_windows;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::TestCodexHarness;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
@@ -155,6 +155,7 @@ fn collect_tool_outputs(bodies: &[Value]) -> Result<HashMap<String, ParsedUnifie
|
||||
async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let builder = test_codex().with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
@@ -279,6 +280,7 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> {
|
||||
async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -350,6 +352,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
async fn unified_exec_resolves_relative_workdir() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -427,6 +430,7 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> {
|
||||
async fn unified_exec_respects_workdir_override() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -505,6 +509,7 @@ async fn unified_exec_respects_workdir_override() -> Result<()> {
|
||||
async fn unified_exec_emits_exec_command_end_event() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -591,6 +596,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> {
|
||||
async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -662,6 +668,7 @@ async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> {
|
||||
async fn unified_exec_emits_output_delta_for_write_stdin() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -761,6 +768,7 @@ async fn unified_exec_emits_output_delta_for_write_stdin() -> Result<()> {
|
||||
async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -857,6 +865,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> {
|
||||
async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -978,6 +987,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()>
|
||||
async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1085,6 +1095,7 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
|
||||
async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1177,6 +1188,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
|
||||
async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1338,6 +1350,7 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
|
||||
async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1442,6 +1455,7 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<()
|
||||
async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1553,6 +1567,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
|
||||
async fn unified_exec_streams_after_lagged_output() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1684,6 +1699,7 @@ PY
|
||||
async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1790,6 +1806,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
|
||||
async fn unified_exec_formats_large_output_summary() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -1875,6 +1892,7 @@ PY
|
||||
async fn unified_exec_runs_under_sandbox() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
@@ -2067,11 +2085,83 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unified_exec_runs_on_all_platforms() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.features.enable(Feature::UnifiedExec);
|
||||
});
|
||||
let TestCodex {
|
||||
codex,
|
||||
cwd,
|
||||
session_configured,
|
||||
..
|
||||
} = builder.build(&server).await?;
|
||||
|
||||
let call_id = "uexec";
|
||||
let args = serde_json::json!({
|
||||
"cmd": "echo 'hello crossplat'",
|
||||
});
|
||||
|
||||
let responses = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
];
|
||||
mount_sse_sequence(&server, responses).await;
|
||||
|
||||
let session_model = session_configured.model.clone();
|
||||
|
||||
codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "summarize large output".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: session_model,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
let requests = server.received_requests().await.expect("recorded requests");
|
||||
assert!(!requests.is_empty(), "expected at least one POST request");
|
||||
|
||||
let bodies = requests
|
||||
.iter()
|
||||
.map(|req| req.body_json::<Value>().expect("request json"))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let outputs = collect_tool_outputs(&bodies)?;
|
||||
let output = outputs.get(call_id).expect("missing output");
|
||||
|
||||
// TODO: Weaker match because windows produces control characters
|
||||
assert_regex_match(".*hello crossplat.*", &output.output);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[ignore]
|
||||
async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
[package]
|
||||
name = "codex-exec-server"
|
||||
version.workspace = true
|
||||
edition.workspace = true
|
||||
license.workspace = true
|
||||
name = "codex-exec-server"
|
||||
version.workspace = true
|
||||
|
||||
[[bin]]
|
||||
name = "codex-execve-wrapper"
|
||||
@@ -56,5 +56,9 @@ tracing = { workspace = true }
|
||||
tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] }
|
||||
|
||||
[dev-dependencies]
|
||||
assert_cmd = { workspace = true }
|
||||
exec_server_test_support = { workspace = true }
|
||||
maplit = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
which = { workspace = true }
|
||||
|
||||
@@ -6,3 +6,6 @@ pub use posix::main_execve_wrapper;
|
||||
|
||||
#[cfg(unix)]
|
||||
pub use posix::main_mcp_server;
|
||||
|
||||
#[cfg(unix)]
|
||||
pub use posix::ExecResult;
|
||||
|
||||
@@ -82,6 +82,8 @@ mod mcp_escalation_policy;
|
||||
mod socket;
|
||||
mod stopwatch;
|
||||
|
||||
pub use mcp::ExecResult;
|
||||
|
||||
/// Default value of --execve option relative to the current executable.
|
||||
/// Note this must match the name of the binary as specified in Cargo.toml.
|
||||
const CODEX_EXECVE_WRAPPER_EXE_NAME: &str = "codex-execve-wrapper";
|
||||
|
||||
@@ -54,7 +54,7 @@ pub struct ExecParams {
|
||||
pub login: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Serialize, schemars::JsonSchema)]
|
||||
#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)]
|
||||
pub struct ExecResult {
|
||||
pub exit_code: i32,
|
||||
pub output: String,
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
// Single integration test binary that aggregates all test modules.
|
||||
// The submodules live in `tests/suite/`.
|
||||
mod suite;
|
||||
@@ -0,0 +1,16 @@
|
||||
[package]
|
||||
name = "exec_server_test_support"
|
||||
version.workspace = true
|
||||
edition.workspace = true
|
||||
license.workspace = true
|
||||
|
||||
[lib]
|
||||
path = "lib.rs"
|
||||
|
||||
[dependencies]
|
||||
assert_cmd = { workspace = true }
|
||||
anyhow = { workspace = true }
|
||||
codex-core = { workspace = true }
|
||||
rmcp = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tokio = { workspace = true }
|
||||
@@ -0,0 +1,167 @@
|
||||
use codex_core::MCP_SANDBOX_STATE_NOTIFICATION;
|
||||
use codex_core::SandboxState;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use rmcp::ClientHandler;
|
||||
use rmcp::ErrorData as McpError;
|
||||
use rmcp::RoleClient;
|
||||
use rmcp::Service;
|
||||
use rmcp::model::ClientCapabilities;
|
||||
use rmcp::model::ClientInfo;
|
||||
use rmcp::model::CreateElicitationRequestParam;
|
||||
use rmcp::model::CreateElicitationResult;
|
||||
use rmcp::model::CustomClientNotification;
|
||||
use rmcp::model::ElicitationAction;
|
||||
use rmcp::service::RunningService;
|
||||
use rmcp::transport::ConfigureCommandExt;
|
||||
use rmcp::transport::TokioChildProcess;
|
||||
use serde_json::json;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Stdio;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use tokio::process::Command;
|
||||
|
||||
pub fn create_transport<P>(codex_home: P) -> anyhow::Result<TokioChildProcess>
|
||||
where
|
||||
P: AsRef<Path>,
|
||||
{
|
||||
let mcp_executable = assert_cmd::Command::cargo_bin("codex-exec-mcp-server")?;
|
||||
let execve_wrapper = assert_cmd::Command::cargo_bin("codex-execve-wrapper")?;
|
||||
let bash = Path::new(env!("CARGO_MANIFEST_DIR"))
|
||||
.join("..")
|
||||
.join("..")
|
||||
.join("tests")
|
||||
.join("suite")
|
||||
.join("bash");
|
||||
|
||||
let transport =
|
||||
TokioChildProcess::new(Command::new(mcp_executable.get_program()).configure(|cmd| {
|
||||
cmd.arg("--bash").arg(bash);
|
||||
cmd.arg("--execve").arg(execve_wrapper.get_program());
|
||||
cmd.env("CODEX_HOME", codex_home.as_ref());
|
||||
|
||||
// Important: pipe stdio so rmcp can speak JSON-RPC over stdin/stdout
|
||||
cmd.stdin(Stdio::piped());
|
||||
cmd.stdout(Stdio::piped());
|
||||
|
||||
// Optional but very helpful while debugging:
|
||||
cmd.stderr(Stdio::inherit());
|
||||
}))?;
|
||||
|
||||
Ok(transport)
|
||||
}
|
||||
|
||||
pub async fn write_default_execpolicy<P>(policy: &str, codex_home: P) -> anyhow::Result<()>
|
||||
where
|
||||
P: AsRef<Path>,
|
||||
{
|
||||
let policy_dir = codex_home.as_ref().join("policy");
|
||||
tokio::fs::create_dir_all(&policy_dir).await?;
|
||||
tokio::fs::write(policy_dir.join("default.codexpolicy"), policy).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn notify_readable_sandbox<P, S>(
|
||||
sandbox_cwd: P,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
service: &RunningService<RoleClient, S>,
|
||||
) -> anyhow::Result<()>
|
||||
where
|
||||
P: AsRef<Path>,
|
||||
S: Service<RoleClient> + ClientHandler,
|
||||
{
|
||||
let sandbox_state = SandboxState {
|
||||
sandbox_policy: SandboxPolicy::ReadOnly,
|
||||
codex_linux_sandbox_exe,
|
||||
sandbox_cwd: sandbox_cwd.as_ref().to_path_buf(),
|
||||
};
|
||||
send_sandbox_notification(sandbox_state, service).await
|
||||
}
|
||||
|
||||
pub async fn notify_writable_sandbox_only_one_folder<P, S>(
|
||||
writable_folder: P,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
service: &RunningService<RoleClient, S>,
|
||||
) -> anyhow::Result<()>
|
||||
where
|
||||
P: AsRef<Path>,
|
||||
S: Service<RoleClient> + ClientHandler,
|
||||
{
|
||||
let sandbox_state = SandboxState {
|
||||
sandbox_policy: SandboxPolicy::WorkspaceWrite {
|
||||
// Note that sandbox_cwd will already be included as a writable root
|
||||
// when the sandbox policy is expanded.
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
// Disable writes to temp dir because this is a test, so
|
||||
// writable_folder is likely also under /tmp and we want to be
|
||||
// strict about what is writable.
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
},
|
||||
codex_linux_sandbox_exe,
|
||||
sandbox_cwd: writable_folder.as_ref().to_path_buf(),
|
||||
};
|
||||
send_sandbox_notification(sandbox_state, service).await
|
||||
}
|
||||
|
||||
async fn send_sandbox_notification<S>(
|
||||
sandbox_state: SandboxState,
|
||||
service: &RunningService<RoleClient, S>,
|
||||
) -> anyhow::Result<()>
|
||||
where
|
||||
S: Service<RoleClient> + ClientHandler,
|
||||
{
|
||||
let sandbox_state_notification = CustomClientNotification::new(
|
||||
MCP_SANDBOX_STATE_NOTIFICATION,
|
||||
Some(serde_json::to_value(sandbox_state)?),
|
||||
);
|
||||
service
|
||||
.send_notification(sandbox_state_notification.into())
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub struct InteractiveClient {
|
||||
pub elicitations_to_accept: HashSet<String>,
|
||||
pub elicitation_requests: Arc<Mutex<Vec<CreateElicitationRequestParam>>>,
|
||||
}
|
||||
|
||||
impl ClientHandler for InteractiveClient {
|
||||
fn get_info(&self) -> ClientInfo {
|
||||
let capabilities = ClientCapabilities::builder().enable_elicitation().build();
|
||||
ClientInfo {
|
||||
capabilities,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn create_elicitation(
|
||||
&self,
|
||||
request: CreateElicitationRequestParam,
|
||||
_context: rmcp::service::RequestContext<RoleClient>,
|
||||
) -> impl std::future::Future<Output = Result<CreateElicitationResult, McpError>> + Send + '_
|
||||
{
|
||||
self.elicitation_requests
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push(request.clone());
|
||||
|
||||
let accept = self.elicitations_to_accept.contains(&request.message);
|
||||
async move {
|
||||
if accept {
|
||||
Ok(CreateElicitationResult {
|
||||
action: ElicitationAction::Accept,
|
||||
content: Some(json!({ "approve": true })),
|
||||
})
|
||||
} else {
|
||||
Ok(CreateElicitationResult {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,124 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
use std::borrow::Cow;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_exec_server::ExecResult;
|
||||
use exec_server_test_support::InteractiveClient;
|
||||
use exec_server_test_support::create_transport;
|
||||
use exec_server_test_support::notify_readable_sandbox;
|
||||
use exec_server_test_support::write_default_execpolicy;
|
||||
use maplit::hashset;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::ServiceExt;
|
||||
use rmcp::model::CallToolRequestParam;
|
||||
use rmcp::model::CallToolResult;
|
||||
use rmcp::model::CreateElicitationRequestParam;
|
||||
use rmcp::model::object;
|
||||
use serde_json::json;
|
||||
use std::os::unix::fs::symlink;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Verify that when using a read-only sandbox and an execpolicy that prompts,
|
||||
/// the proper elicitation is sent. Upon auto-approving the elicitation, the
|
||||
/// command should be run privileged outside the sandbox.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn accept_elicitation_for_prompt_rule() -> Result<()> {
|
||||
// Configure a stdio transport that will launch the MCP server using
|
||||
// $CODEX_HOME with an execpolicy that prompts for `git init` commands.
|
||||
let codex_home = TempDir::new()?;
|
||||
write_default_execpolicy(
|
||||
r#"
|
||||
# Create a rule with `decision = "prompt"` to exercise the elicitation flow.
|
||||
prefix_rule(
|
||||
pattern = ["git", "init"],
|
||||
decision = "prompt",
|
||||
match = [
|
||||
"git init ."
|
||||
],
|
||||
)
|
||||
"#,
|
||||
codex_home.as_ref(),
|
||||
)
|
||||
.await?;
|
||||
let transport = create_transport(codex_home.as_ref())?;
|
||||
|
||||
// Create an MCP client that approves expected elicitation messages.
|
||||
let project_root = TempDir::new()?;
|
||||
let git = which::which("git")?;
|
||||
let project_root_path = project_root.path().canonicalize().unwrap();
|
||||
let expected_elicitation_message = format!(
|
||||
"Allow agent to run `{} init .` in `{}`?",
|
||||
git.display(),
|
||||
project_root_path.display()
|
||||
);
|
||||
let elicitation_requests: Arc<Mutex<Vec<CreateElicitationRequestParam>>> = Default::default();
|
||||
let client = InteractiveClient {
|
||||
elicitations_to_accept: hashset! { expected_elicitation_message.clone() },
|
||||
elicitation_requests: elicitation_requests.clone(),
|
||||
};
|
||||
|
||||
// Start the MCP server.
|
||||
let service: rmcp::service::RunningService<rmcp::RoleClient, InteractiveClient> =
|
||||
client.serve(transport).await?;
|
||||
|
||||
// Notify the MCP server about the current sandbox state before making any
|
||||
// `shell` tool calls.
|
||||
let linux_sandbox_exe_folder = TempDir::new()?;
|
||||
let codex_linux_sandbox_exe = if cfg!(target_os = "linux") {
|
||||
let codex_linux_sandbox_exe = linux_sandbox_exe_folder.path().join("codex-linux-sandbox");
|
||||
let codex_cli = assert_cmd::Command::cargo_bin("codex")?
|
||||
.get_program()
|
||||
.to_os_string();
|
||||
let codex_cli_path = std::path::PathBuf::from(codex_cli);
|
||||
symlink(&codex_cli_path, &codex_linux_sandbox_exe)?;
|
||||
Some(codex_linux_sandbox_exe)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
notify_readable_sandbox(&project_root_path, codex_linux_sandbox_exe, &service).await?;
|
||||
|
||||
// Call the shell tool and verify that an elicitation was created and
|
||||
// auto-approved.
|
||||
let CallToolResult {
|
||||
content, is_error, ..
|
||||
} = service
|
||||
.call_tool(CallToolRequestParam {
|
||||
name: Cow::Borrowed("shell"),
|
||||
arguments: Some(object(json!(
|
||||
{
|
||||
"command": "git init .",
|
||||
"workdir": project_root_path.to_string_lossy(),
|
||||
}
|
||||
))),
|
||||
})
|
||||
.await?;
|
||||
let tool_call_content = content
|
||||
.first()
|
||||
.expect("expected non-empty content")
|
||||
.as_text()
|
||||
.expect("expected text content");
|
||||
let ExecResult {
|
||||
exit_code, output, ..
|
||||
} = serde_json::from_str::<ExecResult>(&tool_call_content.text)?;
|
||||
assert_eq!(
|
||||
output,
|
||||
format!(
|
||||
"Initialized empty Git repository in {}/.git/\n",
|
||||
project_root_path.display()
|
||||
)
|
||||
);
|
||||
assert_eq!(exit_code, 0, "command should succeed");
|
||||
assert_eq!(is_error, Some(false), "command should succeed");
|
||||
|
||||
let elicitation_messages = elicitation_requests
|
||||
.lock()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|r| r.message.clone())
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(vec![expected_elicitation_message], elicitation_messages);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
Executable
+71
@@ -0,0 +1,71 @@
|
||||
#!/usr/bin/env dotslash
|
||||
|
||||
// This is an instance of the fork of Bash that we bundle with
|
||||
// https://www.npmjs.com/package/@openai/codex-shell-tool-mcp.
|
||||
// Fetching the prebuilt version via DotSlash makes it easier to write
|
||||
// integration tests for the MCP server.
|
||||
{
|
||||
"name": "codex-bash",
|
||||
"platforms": {
|
||||
// macOS 13 builds (and therefore x86_64) were dropped in
|
||||
// https://github.com/openai/codex/pull/7295, so we only provide an
|
||||
// Apple Silicon build for now.
|
||||
"macos-aarch64": {
|
||||
"size": 37003612,
|
||||
"hash": "blake3",
|
||||
"digest": "d9cd5928c993b65c340507931c61c02bd6e9179933f8bf26a548482bb5fa53bb",
|
||||
"format": "tar.gz",
|
||||
"path": "package/vendor/aarch64-apple-darwin/bash/macos-15/bash",
|
||||
"providers": [
|
||||
{
|
||||
"url": "https://github.com/openai/codex/releases/download/rust-v0.65.0/codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
},
|
||||
{
|
||||
"type": "github-release",
|
||||
"repo": "openai/codex",
|
||||
"tag": "rust-v0.65.0",
|
||||
"name": "codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
}
|
||||
]
|
||||
},
|
||||
// Note the `musl` parts of the Linux paths are misleading: the Bash
|
||||
// binaries are actually linked against `glibc`, but the
|
||||
// `codex-execve-wrapper` that invokes them is linked against `musl`.
|
||||
"linux-x86_64": {
|
||||
"size": 37003612,
|
||||
"hash": "blake3",
|
||||
"digest": "d9cd5928c993b65c340507931c61c02bd6e9179933f8bf26a548482bb5fa53bb",
|
||||
"format": "tar.gz",
|
||||
"path": "package/vendor/x86_64-unknown-linux-musl/bash/ubuntu-24.04/bash",
|
||||
"providers": [
|
||||
{
|
||||
"url": "https://github.com/openai/codex/releases/download/rust-v0.65.0/codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
},
|
||||
{
|
||||
"type": "github-release",
|
||||
"repo": "openai/codex",
|
||||
"tag": "rust-v0.65.0",
|
||||
"name": "codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
}
|
||||
]
|
||||
},
|
||||
"linux-aarch64": {
|
||||
"size": 37003612,
|
||||
"hash": "blake3",
|
||||
"digest": "d9cd5928c993b65c340507931c61c02bd6e9179933f8bf26a548482bb5fa53bb",
|
||||
"format": "tar.gz",
|
||||
"path": "package/vendor/aarch64-unknown-linux-musl/bash/ubuntu-24.04/bash",
|
||||
"providers": [
|
||||
{
|
||||
"url": "https://github.com/openai/codex/releases/download/rust-v0.65.0/codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
},
|
||||
{
|
||||
"type": "github-release",
|
||||
"repo": "openai/codex",
|
||||
"tag": "rust-v0.65.0",
|
||||
"name": "codex-shell-tool-mcp-npm-0.65.0.tgz"
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,76 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
use std::borrow::Cow;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::Result;
|
||||
use exec_server_test_support::create_transport;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::ServiceExt;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::object;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Verify the list_tools call to the MCP server returns the expected response.
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn list_tools() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let policy_dir = codex_home.path().join("policy");
|
||||
fs::create_dir_all(&policy_dir)?;
|
||||
fs::write(
|
||||
policy_dir.join("default.codexpolicy"),
|
||||
r#"prefix_rule(pattern=["ls"], decision="prompt")"#,
|
||||
)?;
|
||||
let transport = create_transport(codex_home.path())?;
|
||||
|
||||
let service = ().serve(transport).await?;
|
||||
let tools = service.list_tools(Default::default()).await?.tools;
|
||||
assert_eq!(
|
||||
vec![Tool {
|
||||
name: Cow::Borrowed("shell"),
|
||||
title: None,
|
||||
description: Some(Cow::Borrowed(
|
||||
"Runs a shell command and returns its output. You MUST provide the workdir as an absolute path."
|
||||
)),
|
||||
input_schema: Arc::new(object(json!({
|
||||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||
"properties": {
|
||||
"command": {
|
||||
"description": "The bash string to execute.",
|
||||
"type": "string",
|
||||
},
|
||||
"login": {
|
||||
"description": "Launch Bash with -lc instead of -c: defaults to true.",
|
||||
"nullable": true,
|
||||
"type": "boolean",
|
||||
},
|
||||
"timeout_ms": {
|
||||
"description": "The timeout for the command in milliseconds.",
|
||||
"format": "uint64",
|
||||
"minimum": 0,
|
||||
"nullable": true,
|
||||
"type": "integer",
|
||||
},
|
||||
"workdir": {
|
||||
"description": "The working directory to execute the command in. Must be an absolute path.",
|
||||
"type": "string",
|
||||
},
|
||||
},
|
||||
"required": [
|
||||
"command",
|
||||
"workdir",
|
||||
],
|
||||
"title": "ExecParams",
|
||||
"type": "object",
|
||||
}))),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None
|
||||
}],
|
||||
tools
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -0,0 +1,4 @@
|
||||
#[cfg(any(all(target_os = "macos", target_arch = "aarch64"), target_os = "linux"))]
|
||||
mod accept_elicitation;
|
||||
#[cfg(any(all(target_os = "macos", target_arch = "aarch64"), target_os = "linux"))]
|
||||
mod list_tools;
|
||||
@@ -29,7 +29,7 @@ opentelemetry-otlp = { workspace = true, features = [
|
||||
"http-proto",
|
||||
"http-json",
|
||||
"logs",
|
||||
"reqwest",
|
||||
"reqwest-blocking-client",
|
||||
"reqwest-rustls",
|
||||
"tls",
|
||||
"tls-roots",
|
||||
@@ -40,7 +40,7 @@ opentelemetry_sdk = { workspace = true, features = [
|
||||
"rt-tokio",
|
||||
], optional = true }
|
||||
http = { workspace = true }
|
||||
reqwest = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["blocking", "rustls-tls"] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
strum_macros = { workspace = true }
|
||||
|
||||
@@ -182,12 +182,27 @@ fn build_grpc_tls_config(
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
/// Build a blocking HTTP client with TLS configuration for the OTLP HTTP exporter.
|
||||
///
|
||||
/// We use `reqwest::blocking::Client` instead of the async client because the
|
||||
/// `opentelemetry_sdk` `BatchLogProcessor` spawns a dedicated OS thread that uses
|
||||
/// `futures_executor::block_on()` rather than tokio. When the async reqwest client's
|
||||
/// timeout calls `tokio::time::sleep()`, it panics with "no reactor running".
|
||||
fn build_http_client(
|
||||
tls: &OtelTlsConfig,
|
||||
codex_home: &Path,
|
||||
) -> Result<reqwest::Client, Box<dyn Error>> {
|
||||
let mut builder =
|
||||
reqwest::Client::builder().timeout(resolve_otlp_timeout(OTEL_EXPORTER_OTLP_LOGS_TIMEOUT));
|
||||
) -> Result<reqwest::blocking::Client, Box<dyn Error>> {
|
||||
// Wrap in block_in_place because reqwest::blocking::Client creates its own
|
||||
// internal tokio runtime, which would panic if built directly from an async context.
|
||||
tokio::task::block_in_place(|| build_http_client_inner(tls, codex_home))
|
||||
}
|
||||
|
||||
fn build_http_client_inner(
|
||||
tls: &OtelTlsConfig,
|
||||
codex_home: &Path,
|
||||
) -> Result<reqwest::blocking::Client, Box<dyn Error>> {
|
||||
let mut builder = reqwest::blocking::Client::builder()
|
||||
.timeout(resolve_otlp_timeout(OTEL_EXPORTER_OTLP_LOGS_TIMEOUT));
|
||||
|
||||
if let Some(path) = tls.ca_certificate.as_ref() {
|
||||
let (pem, location) = read_bytes(codex_home, path)?;
|
||||
@@ -197,7 +212,10 @@ fn build_http_client(
|
||||
location.display()
|
||||
))
|
||||
})?;
|
||||
builder = builder.add_root_certificate(certificate);
|
||||
// Disable built-in root certificates and use only our custom CA
|
||||
builder = builder
|
||||
.tls_built_in_root_certs(false)
|
||||
.add_root_certificate(certificate);
|
||||
}
|
||||
|
||||
match (&tls.client_certificate, &tls.client_private_key) {
|
||||
@@ -212,7 +230,7 @@ fn build_http_client(
|
||||
key_location.display()
|
||||
))
|
||||
})?;
|
||||
builder = builder.identity(identity);
|
||||
builder = builder.identity(identity).https_only(true);
|
||||
}
|
||||
(Some(_), None) | (None, Some(_)) => {
|
||||
return Err(config_error(
|
||||
|
||||
@@ -348,6 +348,9 @@ pub struct ShellCommandToolCallParams {
|
||||
pub command: String,
|
||||
pub workdir: Option<String>,
|
||||
|
||||
/// Whether to run the shell with login shell semantics
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub login: Option<bool>,
|
||||
/// This is the maximum time in milliseconds that the command is allowed to run.
|
||||
#[serde(alias = "timeout")]
|
||||
pub timeout_ms: Option<u64>,
|
||||
|
||||
@@ -105,6 +105,7 @@ arboard = { workspace = true }
|
||||
|
||||
|
||||
[dev-dependencies]
|
||||
codex-core = { workspace = true, features = ["test-support"] }
|
||||
assert_matches = { workspace = true }
|
||||
chrono = { workspace = true, features = ["serde"] }
|
||||
insta = { workspace = true }
|
||||
|
||||
+19
-2
@@ -302,7 +302,10 @@ impl App {
|
||||
};
|
||||
|
||||
let enhanced_keys_supported = tui.enhanced_keys_supported();
|
||||
|
||||
let model_family = conversation_manager
|
||||
.get_models_manager()
|
||||
.construct_model_family(&config.model, &config)
|
||||
.await;
|
||||
let mut chat_widget = match resume_selection {
|
||||
ResumeSelection::StartFresh | ResumeSelection::Exit => {
|
||||
let init = crate::chatwidget::ChatWidgetInit {
|
||||
@@ -317,6 +320,7 @@ impl App {
|
||||
feedback: feedback.clone(),
|
||||
skills: skills.clone(),
|
||||
is_first_run,
|
||||
model_family,
|
||||
};
|
||||
ChatWidget::new(init, conversation_manager.clone())
|
||||
}
|
||||
@@ -343,6 +347,7 @@ impl App {
|
||||
feedback: feedback.clone(),
|
||||
skills: skills.clone(),
|
||||
is_first_run,
|
||||
model_family,
|
||||
};
|
||||
ChatWidget::new_from_existing(
|
||||
init,
|
||||
@@ -481,6 +486,11 @@ impl App {
|
||||
}
|
||||
|
||||
async fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result<bool> {
|
||||
let model_family = self
|
||||
.server
|
||||
.get_models_manager()
|
||||
.construct_model_family(&self.config.model, &self.config)
|
||||
.await;
|
||||
match event {
|
||||
AppEvent::NewSession => {
|
||||
let summary = session_summary(
|
||||
@@ -500,6 +510,7 @@ impl App {
|
||||
feedback: self.feedback.clone(),
|
||||
skills: self.skills.clone(),
|
||||
is_first_run: false,
|
||||
model_family,
|
||||
};
|
||||
self.chat_widget = ChatWidget::new(init, self.server.clone());
|
||||
if let Some(summary) = summary {
|
||||
@@ -549,6 +560,7 @@ impl App {
|
||||
feedback: self.feedback.clone(),
|
||||
skills: self.skills.clone(),
|
||||
is_first_run: false,
|
||||
model_family: model_family.clone(),
|
||||
};
|
||||
self.chat_widget = ChatWidget::new_from_existing(
|
||||
init,
|
||||
@@ -677,7 +689,12 @@ impl App {
|
||||
self.on_update_reasoning_effort(effort);
|
||||
}
|
||||
AppEvent::UpdateModel(model) => {
|
||||
self.chat_widget.set_model(&model);
|
||||
let model_family = self
|
||||
.server
|
||||
.get_models_manager()
|
||||
.construct_model_family(&model, &self.config)
|
||||
.await;
|
||||
self.chat_widget.set_model(&model, model_family);
|
||||
self.config.model = model;
|
||||
}
|
||||
AppEvent::OpenReasoningPopup { model } => {
|
||||
|
||||
@@ -340,6 +340,7 @@ impl App {
|
||||
let session_configured = new_conv.session_configured;
|
||||
let init = crate::chatwidget::ChatWidgetInit {
|
||||
config: cfg,
|
||||
model_family: self.chat_widget.get_model_family(),
|
||||
frame_requester: tui.frame_requester(),
|
||||
app_event_tx: self.app_event_tx.clone(),
|
||||
initial_prompt: None,
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_core::config::Config;
|
||||
use codex_core::config::types::Notifications;
|
||||
use codex_core::git_info::current_branch_name;
|
||||
use codex_core::git_info::local_git_branches;
|
||||
use codex_core::openai_models::model_family::ModelFamily;
|
||||
use codex_core::openai_models::models_manager::ModelsManager;
|
||||
use codex_core::project_doc::DEFAULT_PROJECT_DOC_FILENAME;
|
||||
use codex_core::protocol::AgentMessageDeltaEvent;
|
||||
@@ -261,6 +262,7 @@ pub(crate) struct ChatWidgetInit {
|
||||
pub(crate) feedback: codex_feedback::CodexFeedback,
|
||||
pub(crate) skills: Option<Vec<SkillMetadata>>,
|
||||
pub(crate) is_first_run: bool,
|
||||
pub(crate) model_family: ModelFamily,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
@@ -277,6 +279,7 @@ pub(crate) struct ChatWidget {
|
||||
bottom_pane: BottomPane,
|
||||
active_cell: Option<Box<dyn HistoryCell>>,
|
||||
config: Config,
|
||||
model_family: ModelFamily,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
models_manager: Arc<ModelsManager>,
|
||||
session_header: SessionHeader,
|
||||
@@ -465,15 +468,13 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
fn on_agent_reasoning_final(&mut self) {
|
||||
let reasoning_summary_format = self.get_model_family().reasoning_summary_format;
|
||||
// At the end of a reasoning block, record transcript-only content.
|
||||
self.full_reasoning_buffer.push_str(&self.reasoning_buffer);
|
||||
let model_family = self
|
||||
.models_manager
|
||||
.construct_model_family(&self.config.model, &self.config);
|
||||
if !self.full_reasoning_buffer.is_empty() {
|
||||
let cell = history_cell::new_reasoning_summary_block(
|
||||
self.full_reasoning_buffer.clone(),
|
||||
&model_family,
|
||||
reasoning_summary_format,
|
||||
);
|
||||
self.add_boxed_history(cell);
|
||||
}
|
||||
@@ -647,6 +648,9 @@ impl ChatWidget {
|
||||
self.stream_controller = None;
|
||||
self.maybe_show_pending_rate_limit_prompt();
|
||||
}
|
||||
pub(crate) fn get_model_family(&self) -> ModelFamily {
|
||||
self.model_family.clone()
|
||||
}
|
||||
|
||||
fn on_error(&mut self, message: String) {
|
||||
self.finalize_turn();
|
||||
@@ -1249,6 +1253,7 @@ impl ChatWidget {
|
||||
feedback,
|
||||
skills,
|
||||
is_first_run,
|
||||
model_family,
|
||||
} = common;
|
||||
let mut rng = rand::rng();
|
||||
let placeholder = EXAMPLE_PROMPTS[rng.random_range(0..EXAMPLE_PROMPTS.len())].to_string();
|
||||
@@ -1270,6 +1275,7 @@ impl ChatWidget {
|
||||
}),
|
||||
active_cell: None,
|
||||
config: config.clone(),
|
||||
model_family,
|
||||
auth_manager,
|
||||
models_manager,
|
||||
session_header: SessionHeader::new(config.model),
|
||||
@@ -1329,6 +1335,7 @@ impl ChatWidget {
|
||||
models_manager,
|
||||
feedback,
|
||||
skills,
|
||||
model_family,
|
||||
..
|
||||
} = common;
|
||||
let mut rng = rand::rng();
|
||||
@@ -1353,6 +1360,7 @@ impl ChatWidget {
|
||||
}),
|
||||
active_cell: None,
|
||||
config: config.clone(),
|
||||
model_family,
|
||||
auth_manager,
|
||||
models_manager,
|
||||
session_header: SessionHeader::new(config.model),
|
||||
@@ -1785,7 +1793,7 @@ impl ChatWidget {
|
||||
EventMsg::AgentReasoning(AgentReasoningEvent { .. }) => self.on_agent_reasoning_final(),
|
||||
EventMsg::AgentReasoningRawContent(AgentReasoningRawContentEvent { text }) => {
|
||||
self.on_agent_reasoning_delta(text);
|
||||
self.on_agent_reasoning_final()
|
||||
self.on_agent_reasoning_final();
|
||||
}
|
||||
EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(),
|
||||
EventMsg::TaskStarted(_) => self.on_task_started(),
|
||||
@@ -2843,9 +2851,10 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
/// Set the model in the widget's config copy.
|
||||
pub(crate) fn set_model(&mut self, model: &str) {
|
||||
pub(crate) fn set_model(&mut self, model: &str, model_family: ModelFamily) {
|
||||
self.session_header.set_model(model);
|
||||
self.config.model = model.to_string();
|
||||
self.model_family = model_family;
|
||||
}
|
||||
|
||||
pub(crate) fn add_info_message(&mut self, message: String, hint: Option<String>) {
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_core::CodexAuth;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::ConfigToml;
|
||||
use codex_core::openai_models::models_manager::ModelsManager;
|
||||
use codex_core::protocol::AgentMessageDeltaEvent;
|
||||
use codex_core::protocol::AgentMessageEvent;
|
||||
use codex_core::protocol::AgentReasoningDeltaEvent;
|
||||
@@ -345,6 +346,7 @@ async fn helpers_are_available_and_do_not_panic() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let cfg = test_config();
|
||||
let model_family = ModelsManager::construct_model_family_offline(&cfg.model, &cfg);
|
||||
let conversation_manager = Arc::new(ConversationManager::with_auth(CodexAuth::from_api_key(
|
||||
"test",
|
||||
)));
|
||||
@@ -361,6 +363,7 @@ async fn helpers_are_available_and_do_not_panic() {
|
||||
feedback: codex_feedback::CodexFeedback::new(),
|
||||
skills: None,
|
||||
is_first_run: true,
|
||||
model_family,
|
||||
};
|
||||
let mut w = ChatWidget::new(init, conversation_manager);
|
||||
// Basic construction sanity.
|
||||
@@ -394,6 +397,7 @@ fn make_chatwidget_manual() -> (
|
||||
bottom_pane: bottom,
|
||||
active_cell: None,
|
||||
config: cfg.clone(),
|
||||
model_family: ModelsManager::construct_model_family_offline(&cfg.model, &cfg),
|
||||
auth_manager: auth_manager.clone(),
|
||||
models_manager: Arc::new(ModelsManager::new(auth_manager)),
|
||||
session_header: SessionHeader::new(cfg.model),
|
||||
|
||||
@@ -27,7 +27,6 @@ use codex_common::format_env_display::format_env_display;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::types::McpServerTransportConfig;
|
||||
use codex_core::config::types::ReasoningSummaryFormat;
|
||||
use codex_core::openai_models::model_family::ModelFamily;
|
||||
use codex_core::protocol::FileChange;
|
||||
use codex_core::protocol::McpAuthStatus;
|
||||
use codex_core::protocol::McpInvocation;
|
||||
@@ -1421,9 +1420,9 @@ pub(crate) fn new_view_image_tool_call(path: PathBuf, cwd: &Path) -> PlainHistor
|
||||
|
||||
pub(crate) fn new_reasoning_summary_block(
|
||||
full_reasoning_buffer: String,
|
||||
model_family: &ModelFamily,
|
||||
reasoning_summary_format: ReasoningSummaryFormat,
|
||||
) -> Box<dyn HistoryCell> {
|
||||
if model_family.reasoning_summary_format == ReasoningSummaryFormat::Experimental {
|
||||
if reasoning_summary_format == ReasoningSummaryFormat::Experimental {
|
||||
// Experimental format is following:
|
||||
// ** header **
|
||||
//
|
||||
@@ -1513,8 +1512,6 @@ mod tests {
|
||||
use crate::exec_cell::CommandOutput;
|
||||
use crate::exec_cell::ExecCall;
|
||||
use crate::exec_cell::ExecCell;
|
||||
use codex_core::AuthManager;
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::ConfigToml;
|
||||
@@ -1527,7 +1524,6 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::protocol::ExecCommandSource;
|
||||
use mcp_types::CallToolResult;
|
||||
@@ -2326,13 +2322,12 @@ mod tests {
|
||||
#[test]
|
||||
fn reasoning_summary_block() {
|
||||
let config = test_config();
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let reasoning_format =
|
||||
ModelsManager::construct_model_family_offline(&config.model, &config)
|
||||
.reasoning_summary_format;
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level reasoning**\n\nDetailed reasoning goes here.".to_string(),
|
||||
&model_family,
|
||||
reasoning_format,
|
||||
);
|
||||
|
||||
let rendered_display = render_lines(&cell.display_lines(80));
|
||||
@@ -2345,12 +2340,13 @@ mod tests {
|
||||
#[test]
|
||||
fn reasoning_summary_block_returns_reasoning_cell_when_feature_disabled() {
|
||||
let config = test_config();
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let cell =
|
||||
new_reasoning_summary_block("Detailed reasoning goes here.".to_string(), &model_family);
|
||||
let reasoning_format =
|
||||
ModelsManager::construct_model_family_offline(&config.model, &config)
|
||||
.reasoning_summary_format;
|
||||
let cell = new_reasoning_summary_block(
|
||||
"Detailed reasoning goes here.".to_string(),
|
||||
reasoning_format,
|
||||
);
|
||||
|
||||
let rendered = render_transcript(cell.as_ref());
|
||||
assert_eq!(rendered, vec!["• Detailed reasoning goes here."]);
|
||||
@@ -2362,11 +2358,7 @@ mod tests {
|
||||
config.model = "gpt-3.5-turbo".to_string();
|
||||
config.model_supports_reasoning_summaries = Some(true);
|
||||
config.model_reasoning_summary_format = Some(ReasoningSummaryFormat::Experimental);
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let model_family = ModelsManager::construct_model_family_offline(&config.model, &config);
|
||||
assert_eq!(
|
||||
model_family.reasoning_summary_format,
|
||||
ReasoningSummaryFormat::Experimental
|
||||
@@ -2374,7 +2366,7 @@ mod tests {
|
||||
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level reasoning**\n\nDetailed reasoning goes here.".to_string(),
|
||||
&model_family,
|
||||
model_family.reasoning_summary_format,
|
||||
);
|
||||
|
||||
let rendered_display = render_lines(&cell.display_lines(80));
|
||||
@@ -2384,13 +2376,12 @@ mod tests {
|
||||
#[test]
|
||||
fn reasoning_summary_block_falls_back_when_header_is_missing() {
|
||||
let config = test_config();
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let reasoning_format =
|
||||
ModelsManager::construct_model_family_offline(&config.model, &config)
|
||||
.reasoning_summary_format;
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level reasoning without closing".to_string(),
|
||||
&model_family,
|
||||
reasoning_format,
|
||||
);
|
||||
|
||||
let rendered = render_transcript(cell.as_ref());
|
||||
@@ -2400,13 +2391,12 @@ mod tests {
|
||||
#[test]
|
||||
fn reasoning_summary_block_falls_back_when_summary_is_missing() {
|
||||
let config = test_config();
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let reasoning_format =
|
||||
ModelsManager::construct_model_family_offline(&config.model, &config)
|
||||
.reasoning_summary_format;
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level reasoning without closing**".to_string(),
|
||||
&model_family,
|
||||
reasoning_format.clone(),
|
||||
);
|
||||
|
||||
let rendered = render_transcript(cell.as_ref());
|
||||
@@ -2414,7 +2404,7 @@ mod tests {
|
||||
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level reasoning without closing**\n\n ".to_string(),
|
||||
&model_family,
|
||||
reasoning_format,
|
||||
);
|
||||
|
||||
let rendered = render_transcript(cell.as_ref());
|
||||
@@ -2424,13 +2414,12 @@ mod tests {
|
||||
#[test]
|
||||
fn reasoning_summary_block_splits_header_and_summary_when_present() {
|
||||
let config = test_config();
|
||||
let auth_manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
|
||||
let models_manager = Arc::new(ModelsManager::new(auth_manager));
|
||||
let model_family = models_manager.construct_model_family(&config.model, &config);
|
||||
let reasoning_format =
|
||||
ModelsManager::construct_model_family_offline(&config.model, &config)
|
||||
.reasoning_summary_format;
|
||||
let cell = new_reasoning_summary_block(
|
||||
"**High level plan**\n\nWe should fix the bug next.".to_string(),
|
||||
&model_family,
|
||||
reasoning_format,
|
||||
);
|
||||
|
||||
let rendered_display = render_lines(&cell.display_lines(80));
|
||||
|
||||
@@ -10,4 +10,4 @@ workspace = true
|
||||
[dependencies]
|
||||
anyhow = { workspace = true }
|
||||
portable-pty = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync"] }
|
||||
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] }
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use core::fmt;
|
||||
use std::collections::HashMap;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
@@ -9,13 +10,20 @@ use std::time::Duration;
|
||||
use anyhow::Result;
|
||||
use portable_pty::native_pty_system;
|
||||
use portable_pty::CommandBuilder;
|
||||
use portable_pty::MasterPty;
|
||||
use portable_pty::PtySize;
|
||||
use portable_pty::SlavePty;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::sync::Mutex as TokioMutex;
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
pub struct PtyPairWrapper {
|
||||
pub _slave: Option<Box<dyn SlavePty + Send>>,
|
||||
pub _master: Box<dyn MasterPty + Send>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ExecCommandSession {
|
||||
writer_tx: mpsc::Sender<Vec<u8>>,
|
||||
@@ -26,6 +34,15 @@ pub struct ExecCommandSession {
|
||||
wait_handle: StdMutex<Option<JoinHandle<()>>>,
|
||||
exit_status: Arc<AtomicBool>,
|
||||
exit_code: Arc<StdMutex<Option<i32>>>,
|
||||
// PtyPair must be preserved because the process will receive Control+C if the
|
||||
// slave is closed
|
||||
_pair: StdMutex<PtyPairWrapper>,
|
||||
}
|
||||
|
||||
impl fmt::Debug for PtyPairWrapper {
|
||||
fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecCommandSession {
|
||||
@@ -39,6 +56,7 @@ impl ExecCommandSession {
|
||||
wait_handle: JoinHandle<()>,
|
||||
exit_status: Arc<AtomicBool>,
|
||||
exit_code: Arc<StdMutex<Option<i32>>>,
|
||||
pair: PtyPairWrapper,
|
||||
) -> (Self, broadcast::Receiver<Vec<u8>>) {
|
||||
let initial_output_rx = output_tx.subscribe();
|
||||
(
|
||||
@@ -51,6 +69,7 @@ impl ExecCommandSession {
|
||||
wait_handle: StdMutex::new(Some(wait_handle)),
|
||||
exit_status,
|
||||
exit_code,
|
||||
_pair: StdMutex::new(pair),
|
||||
},
|
||||
initial_output_rx,
|
||||
)
|
||||
@@ -192,6 +211,16 @@ pub async fn spawn_pty_process(
|
||||
let _ = exit_tx.send(code);
|
||||
});
|
||||
|
||||
let pair = PtyPairWrapper {
|
||||
_slave: if cfg!(windows) {
|
||||
// Keep the slave handle alive on Windows to prevent the process from receiving Control+C
|
||||
Some(pair.slave)
|
||||
} else {
|
||||
None
|
||||
},
|
||||
_master: pair.master,
|
||||
};
|
||||
|
||||
let (session, output_rx) = ExecCommandSession::new(
|
||||
writer_tx,
|
||||
output_tx,
|
||||
@@ -201,6 +230,7 @@ pub async fn spawn_pty_process(
|
||||
wait_handle,
|
||||
exit_status,
|
||||
exit_code,
|
||||
pair,
|
||||
);
|
||||
|
||||
Ok(SpawnedPty {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
set working-directory := "codex-rs"
|
||||
set positional-arguments
|
||||
|
||||
# Display help
|
||||
Reference in New Issue
Block a user