openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-04 15:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify goal file materialization

Browse Source
This commit is contained in:
Eric Traut
2026-06-10 17:22:07 -07:00
parent 651878b052
commit 807ededaa3
10 changed files with 253 additions and 603 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
codex-rs/app-server-client/src/lib.rs
View File

@@ -38,7 +38,6 @@ use codex_app_server_protocol::ConfigWarningNotification;
use codex_app_server_protocol::InitializeCapabilities;
use codex_app_server_protocol::InitializeParams;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::JSONRPCRequest;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::Result as JsonRpcResult;
use codex_app_server_protocol::ServerNotification;
@@ -848,16 +847,6 @@ impl AppServerClient {
}
}
pub async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult<RequestResult> {
match self {
Self::InProcess(_) => Err(IoError::new(
ErrorKind::InvalidInput,
"raw JSON-RPC requests are only supported by the remote app-server client",
)),
Self::Remote(client) => client.request_json_rpc(request).await,
}
}
pub async fn request_typed<T>(&self, request: ClientRequest) -> Result<T, TypedRequestError>
where
T: DeserializeOwned,
@@ -911,20 +900,6 @@ impl AppServerClient {
}
}
pub fn remote_codex_home(&self) -> Option<&str> {
match self {
Self::InProcess(_) => None,
Self::Remote(client) => client.codex_home(),
}
}
pub fn remote_platform_family(&self) -> Option<&str> {
match self {
Self::InProcess(_) => None,
Self::Remote(client) => client.platform_family(),
}
}
pub fn request_handle(&self) -> AppServerRequestHandle {
match self {
Self::InProcess(client) => AppServerRequestHandle::InProcess(client.request_handle()),
@@ -1131,8 +1106,6 @@ mod tests {
result: serde_json::json!({
"userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust",
"codexHome": "/server/.codex",
"platformFamily": "unix",
"platformOs": "linux",
}),
}),
)
@@ -1470,7 +1443,6 @@ mod tests {
assert_eq!(client.server_version(), Some("9.8.7-test"));
assert_eq!(client.codex_home(), Some("/server/.codex"));
assert_eq!(client.platform_family(), Some("unix"));
let response: GetAccountResponse = client
.request_typed(ClientRequest::GetAccount {
request_id: RequestId::Integer(1),

90
codex-rs/app-server-client/src/remote.rs
View File

@@ -152,7 +152,6 @@ pub struct RemoteAppServerClient {
pending_events: VecDeque<AppServerEvent>,
server_version: Option<String>,
codex_home: Option<String>,
platform_family: Option<String>,
worker_handle: tokio::task::JoinHandle<()>,
}
@@ -160,7 +159,6 @@ pub struct RemoteAppServerClient {
struct RemoteInitializeInfo {
server_version: Option<String>,
codex_home: Option<String>,
platform_family: Option<String>,
}
#[derive(Clone)]
@@ -198,10 +196,6 @@ impl RemoteAppServerClient {
self.codex_home.as_deref()
}
pub fn platform_family(&self) -> Option<&str> {
self.platform_family.as_deref()
}
async fn connect_with_stream<S>(
channel_capacity: usize,
endpoint: String,
@@ -235,17 +229,36 @@ impl RemoteAppServerClient {
};
match command {
RemoteClientCommand::Request { request, response_tx } => {
if !write_remote_request(
let request_id = request.id.clone();
if pending_requests.contains_key(&request_id) {
let _ = response_tx.send(Err(IoError::new(
ErrorKind::InvalidInput,
format!("duplicate remote app-server request id `{request_id}`"),
)));
continue;
}
pending_requests.insert(request_id.clone(), response_tx);
if let Err(err) = write_jsonrpc_message(
&mut stream,
JSONRPCMessage::Request(*request),
&endpoint,
&mut pending_requests,
*request,
response_tx,
&event_tx,
&mut worker_exit_error,
)
.await
{
let err_message = err.to_string();
let message = format!(
"remote app server at `{endpoint}` write failed: {err_message}"
);
if let Some(response_tx) = pending_requests.remove(&request_id) {
let _ = response_tx.send(Err(err));
}
let _ = deliver_event(
&event_tx,
AppServerEvent::Disconnected {
message: message.clone(),
},
);
worker_exit_error = Some((ErrorKind::BrokenPipe, message));
break;
}
}
@@ -471,7 +484,6 @@ impl RemoteAppServerClient {
pending_events: pending_events.into(),
server_version: initialize_info.server_version,
codex_home: initialize_info.codex_home,
platform_family: initialize_info.platform_family,
worker_handle,
})
}
@@ -487,7 +499,7 @@ impl RemoteAppServerClient {
.await
}
pub async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult<RequestResult> {
async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult<RequestResult> {
let (response_tx, response_rx) = oneshot::channel();
self.command_tx
.send(RemoteClientCommand::Request {
@@ -619,7 +631,6 @@ impl RemoteAppServerClient {
pending_events: _pending_events,
server_version: _server_version,
codex_home: _codex_home,
platform_family: _platform_family,
worker_handle,
} = self;
let mut worker_handle = worker_handle;
@@ -648,7 +659,7 @@ impl RemoteAppServerRequestHandle {
.await
}
async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult<RequestResult> {
pub async fn request_json_rpc(&self, request: JSONRPCRequest) -> IoResult<RequestResult> {
let (response_tx, response_rx) = oneshot::channel();
self.command_tx
.send(RemoteClientCommand::Request {
@@ -856,11 +867,6 @@ where
.and_then(serde_json::Value::as_str)
.filter(|codex_home| !codex_home.is_empty())
.map(str::to_string);
initialize_info.platform_family = response
.result
.get("platformFamily")
.and_then(serde_json::Value::as_str)
.map(str::to_string);
break Ok(());
}
JSONRPCMessage::Error(error) if error.id == initialize_request_id => {
@@ -974,47 +980,6 @@ fn deliver_event(
})
}
async fn write_remote_request<S>(
stream: &mut WebSocketStream<S>,
endpoint: &str,
pending_requests: &mut HashMap<RequestId, oneshot::Sender<IoResult<RequestResult>>>,
request: JSONRPCRequest,
response_tx: oneshot::Sender<IoResult<RequestResult>>,
event_tx: &mpsc::UnboundedSender<AppServerEvent>,
worker_exit_error: &mut Option<(ErrorKind, String)>,
) -> bool
where
S: AsyncRead + AsyncWrite + Unpin,
{
let request_id = request.id.clone();
if pending_requests.contains_key(&request_id) {
let _ = response_tx.send(Err(IoError::new(
ErrorKind::InvalidInput,
format!("duplicate remote app-server request id `{request_id}`"),
)));
return true;
}
pending_requests.insert(request_id.clone(), response_tx);
if let Err(err) =
write_jsonrpc_message(stream, JSONRPCMessage::Request(request), endpoint).await
{
let err_message = err.to_string();
let message = format!("remote app server at `{endpoint}` write failed: {err_message}");
if let Some(response_tx) = pending_requests.remove(&request_id) {
let _ = response_tx.send(Err(err));
}
let _ = deliver_event(
event_tx,
AppServerEvent::Disconnected {
message: message.clone(),
},
);
*worker_exit_error = Some((ErrorKind::BrokenPipe, message));
return false;
}
true
}
fn jsonrpc_request_from_client_request(request: ClientRequest) -> JSONRPCRequest {
let value = match serde_json::to_value(request) {
Ok(value) => value,
@@ -1085,7 +1050,6 @@ mod tests {
pending_events: VecDeque::new(),
server_version: None,
codex_home: None,
platform_family: None,
worker_handle,
};

2
codex-rs/tui/src/app/thread_goal_actions.rs
View File

@@ -290,7 +290,7 @@ impl App {
fn show_replace_thread_goal_confirmation(&mut self, thread_id: ThreadId, objective: String) {
let replace_objective = objective.clone();
let subtitle = if let Some(path) = goal_files::objective_file_path(&objective) {
format!("New objective file: {}", path.display())
format!("New objective file: {path}")
} else {
format!("New objective: {objective}")
};

95
codex-rs/tui/src/app_server_session.rs
View File

@@ -12,8 +12,6 @@ use crate::session_state::ThreadSessionState;
use crate::status::StatusAccountDisplay;
use crate::status::plan_type_display_name;
use crate::terminal_visualization_instructions::with_terminal_visualization_instructions;
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
use codex_app_server_client::AppServerClient;
use codex_app_server_client::AppServerEvent;
use codex_app_server_client::AppServerRequestHandle;
@@ -29,17 +27,10 @@ use codex_app_server_protocol::ExternalAgentConfigDetectResponse;
use codex_app_server_protocol::ExternalAgentConfigImportParams;
use codex_app_server_protocol::ExternalAgentConfigImportResponse;
use codex_app_server_protocol::ExternalAgentConfigMigrationItem;
use codex_app_server_protocol::FsCreateDirectoryParams;
use codex_app_server_protocol::FsCreateDirectoryResponse;
use codex_app_server_protocol::FsReadFileParams;
use codex_app_server_protocol::FsReadFileResponse;
use codex_app_server_protocol::FsWriteFileParams;
use codex_app_server_protocol::FsWriteFileResponse;
use codex_app_server_protocol::GetAccountParams;
use codex_app_server_protocol::GetAccountRateLimitsResponse;
use codex_app_server_protocol::GetAccountResponse;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::JSONRPCRequest;
use codex_app_server_protocol::LogoutAccountResponse;
use codex_app_server_protocol::MemoryResetResponse;
use codex_app_server_protocol::Model as ApiModel;
@@ -135,7 +126,6 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use color_eyre::eyre::ContextCompat;
use color_eyre::eyre::Result;
use color_eyre::eyre::WrapErr;
use serde::de::DeserializeOwned;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::atomic::AtomicBool;
@@ -256,11 +246,10 @@ impl AppServerSession {
}
pub(crate) fn remote_codex_home(&self) -> Option<&str> {
self.client.remote_codex_home()
}
pub(crate) fn remote_platform_family(&self) -> Option<&str> {
self.client.remote_platform_family()
let AppServerClient::Remote(client) = &self.client else {
return None;
};
client.codex_home()
}
pub(crate) fn server_version(&self) -> Option<&str> {
@@ -942,82 +931,6 @@ impl AppServerSession {
.wrap_err("thread/goal/clear failed in TUI")
}
pub(crate) async fn fs_read_file(&mut self, path: AbsolutePathBuf) -> Result<Vec<u8>> {
let request_id = self.next_request_id();
let response: FsReadFileResponse = self
.client
.request_typed(ClientRequest::FsReadFile {
request_id,
params: FsReadFileParams { path },
})
.await
.wrap_err("fs/readFile failed in TUI")?;
STANDARD
.decode(response.data_base64)
.wrap_err("fs/readFile returned invalid base64 data")
}
pub(crate) async fn fs_write_file(
&mut self,
path: AbsolutePathBuf,
bytes: Vec<u8>,
) -> Result<()> {
let request_id = self.next_request_id();
let _: FsWriteFileResponse = self
.client
.request_typed(ClientRequest::FsWriteFile {
request_id,
params: FsWriteFileParams {
path,
data_base64: STANDARD.encode(bytes),
},
})
.await
.wrap_err("fs/writeFile failed in TUI")?;
Ok(())
}
pub(crate) async fn fs_create_directory(&mut self, path: AbsolutePathBuf) -> Result<()> {
let request_id = self.next_request_id();
let _: FsCreateDirectoryResponse = self
.client
.request_typed(ClientRequest::FsCreateDirectory {
request_id,
params: FsCreateDirectoryParams {
path,
recursive: Some(true),
},
})
.await
.wrap_err("fs/createDirectory failed in TUI")?;
Ok(())
}
pub(crate) async fn request_json_rpc_typed<T>(
&mut self,
method: &str,
params: serde_json::Value,
) -> Result<T>
where
T: DeserializeOwned,
{
let request_id = self.next_request_id();
let response = self
.client
.request_json_rpc(JSONRPCRequest {
id: request_id,
method: method.to_string(),
params: Some(params),
trace: None,
})
.await
.wrap_err_with(|| format!("{method} failed in TUI"))?;
let result = response.map_err(|source| {
color_eyre::eyre::eyre!("{method} failed in TUI: {}", source.message)
})?;
serde_json::from_value(result).wrap_err_with(|| format!("{method} returned invalid data"))
}
pub(crate) async fn logout_account(&mut self) -> Result<()> {
let request_id = self.next_request_id();
let _: LogoutAccountResponse = self

7
codex-rs/tui/src/chatwidget/goal_menu.rs
View File

@@ -47,7 +47,7 @@ impl ChatWidget {
objective: String,
) {
let subtitle = if let Some(path) = goal_files::objective_file_path(&objective) {
format!("Goal file: {}", path.display())
format!("Goal file: {path}")
} else {
format!("Goal: {objective}")
};
@@ -131,10 +131,7 @@ fn goal_summary_lines(goal: &AppThreadGoal) -> Vec<Line<'static>> {
fn goal_objective_line(objective: &str) -> Line<'static> {
if let Some(path) = goal_files::objective_file_path(objective) {
Line::from(vec![
"Objective file: ".dim(),
path.display().to_string().into(),
])
Line::from(vec!["Objective file: ".dim(), path.into()])
} else {
Line::from(vec!["Objective: ".dim(), objective.to_string().into()])
}

2
codex-rs/tui/src/chatwidget/tests/goal_menu.rs
View File

@@ -84,7 +84,7 @@ async fn goal_menu_managed_file_snapshot() {
.join("attachments")
.join("00000000-0000-4000-8000-000000000000")
.join("goal-objective.md");
let goal_path = crate::goal_files::GoalFilePath::from_local(&path);
let goal_path = path.display().to_string();
goal.objective = crate::goal_files::objective_file_reference(&goal_path)
.expect("goal objective file reference");

58
codex-rs/tui/src/chatwidget/tests/goal_validation.rs
View File

@@ -14,12 +14,6 @@ fn complete_turn_with_message(chat: &mut ChatWidget, turn_id: &str, message: Opt
handle_turn_completed(chat, turn_id, /*duration_ms*/ None);
}
fn submit_composer_text(chat: &mut ChatWidget, text: &str) {
chat.bottom_pane
.set_composer_text(text.to_string(), Vec::new(), Vec::new());
submit_current_composer(chat);
}
fn submit_current_composer(chat: &mut ChatWidget) {
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
@@ -48,46 +42,6 @@ fn next_goal_draft(
}
}
#[test]
fn sentinel_like_objective_is_plain_text() {
let objective = concat!(
"Codex goal objective file: ",
"/tmp/attachments/00000000-0000-4000-8000-000000000000/goal-objective.md\n",
"Read that file before continuing."
);
assert_eq!(crate::goal_files::objective_file_path(objective), None);
}
#[tokio::test]
async fn goal_slash_command_accepts_objective_at_limit() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
let thread_id = ThreadId::new();
chat.thread_id = Some(thread_id);
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS);
let command = format!("/goal {objective}");
submit_composer_text(&mut chat, &command);
assert_eq!(next_goal_draft(&mut rx, thread_id).objective, objective);
assert_no_submit_op(&mut op_rx);
}
#[tokio::test]
async fn goal_slash_command_accepts_multiline_objective_after_blank_first_line() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
let thread_id = ThreadId::new();
chat.thread_id = Some(thread_id);
let objective = "follow these instructions\npreserve this detail";
submit_composer_text(&mut chat, &format!("/goal \n\n{objective}"));
assert_eq!(next_goal_draft(&mut rx, thread_id).objective, objective);
assert_no_submit_op(&mut op_rx);
}
#[tokio::test]
async fn goal_slash_command_emits_only_inserted_paste_text_element() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
@@ -154,16 +108,8 @@ async fn queued_goal_slash_command_emits_oversized_objective_and_stops_queue() {
complete_turn_with_message(&mut chat, "turn-1", Some("done"));
let (actual_thread_id, actual_objective) = loop {
match rx.try_recv().expect("expected goal objective event") {
AppEvent::SetThreadGoalDraft {
thread_id, draft, ..
} => break (thread_id, draft.objective),
_ => continue,
}
};
assert_eq!(actual_thread_id, thread_id);
assert_eq!(actual_objective, objective);
let draft = next_goal_draft(&mut rx, thread_id);
assert_eq!(draft.objective, objective);
assert_eq!(chat.input_queue.queued_user_messages.len(), 1);
assert_no_submit_op(&mut op_rx);
}

25
codex-rs/tui/src/goal_display.rs
View File

@@ -43,7 +43,7 @@ pub(crate) fn goal_status_label(status: ThreadGoalStatus) -> &'static str {
pub(crate) fn goal_usage_summary(goal: &ThreadGoal) -> String {
let objective = if let Some(path) = crate::goal_files::objective_file_path(&goal.objective) {
format!("Objective file: {}", path.display())
format!("Objective file: {path}")
} else {
format!("Objective: {}", goal.objective)
};
@@ -113,27 +113,4 @@ mod tests {
"Objective: Complete the task described in ../gameboy-long-running-prompt5.txt Time: 2m. Tokens: 63.9K/50K."
);
}
#[test]
fn goal_usage_summary_formats_managed_file_objective() {
let temp_dir = tempfile::tempdir().expect("tempdir");
let path = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path_checked(
temp_dir
.path()
.join("attachments")
.join("00000000-0000-4000-8000-000000000000")
.join("goal-objective.md"),
)
.expect("absolute path");
let path_for_goal = crate::goal_files::GoalFilePath::from_local(&path);
let objective = crate::goal_files::objective_file_reference(&path_for_goal)
.expect("goal file reference");
let mut goal = test_thread_goal(/*token_budget*/ None, /*tokens_used*/ 0);
goal.objective = objective;
assert_eq!(
goal_usage_summary(&goal),
format!("Objective file: {} Time: 2m.", path.display())
);
}
}

362
codex-rs/tui/src/goal_files.rs
View File

@@ -15,12 +15,20 @@ use anyhow::Result;
use anyhow::bail;
use base64::Engine;
use base64::engine::general_purpose::STANDARD;
use codex_app_server_client::AppServerRequestHandle;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::FsCreateDirectoryParams;
use codex_app_server_protocol::FsCreateDirectoryResponse;
use codex_app_server_protocol::FsReadFileParams;
use codex_app_server_protocol::FsReadFileResponse;
use codex_app_server_protocol::FsWriteFileParams;
use codex_app_server_protocol::FsWriteFileResponse;
use codex_app_server_protocol::JSONRPCRequest;
use codex_app_server_protocol::RequestId;
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
use codex_protocol::user_input::TextElement;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::de::DeserializeOwned;
use serde_json::json;
use uuid::Uuid;
@@ -60,167 +68,166 @@ pub(crate) trait GoalFileStore {
) -> impl std::future::Future<Output = Result<Vec<u8>>> + Send;
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct GoalFilePath {
raw: String,
separator: char,
}
impl GoalFilePath {
pub(crate) fn from_local(path: &AbsolutePathBuf) -> Self {
Self {
raw: path.display().to_string(),
separator: std::path::MAIN_SEPARATOR,
}
}
pub(crate) fn from_remote(path: &str, platform_family: Option<&str>) -> Self {
Self {
raw: path.to_string(),
separator: if platform_family == Some("windows") || is_windows_absolute_path(path) {
'\\'
} else {
'/'
},
}
}
pub(crate) fn as_str(&self) -> &str {
&self.raw
}
pub(crate) fn display(&self) -> &str {
&self.raw
}
fn join(&self, segment: impl AsRef<str>) -> Self {
let segment = segment.as_ref();
let trimmed = self.raw.trim_end_matches(['/', '\\']);
let mut raw = if trimmed.is_empty() {
self.separator.to_string()
} else {
trimmed.to_string()
};
if !raw.ends_with(self.separator) {
raw.push(self.separator);
}
raw.push_str(segment);
Self {
raw,
separator: self.separator,
}
}
fn from_managed_reference(raw: &str) -> Option<Self> {
let separator = if is_windows_absolute_path(raw) {
'\\'
} else if raw.starts_with('/') {
'/'
} else {
return None;
};
let normalized = raw.replace('\\', "/");
let parts = normalized
.split('/')
.filter(|part| !part.is_empty())
.collect::<Vec<_>>();
if parts.len() < 3 {
return None;
}
let file_name = parts.last()?;
let attachment_id = parts.get(parts.len() - 2)?;
let attachment_dir = parts.get(parts.len() - 3)?;
if *file_name == GOAL_FILE_NAME
&& *attachment_dir == GOAL_ATTACHMENT_DIR
&& Uuid::parse_str(attachment_id).is_ok()
{
Some(Self {
raw: raw.to_string(),
separator,
})
} else {
None
}
}
}
/// Path syntax for goal files has to match the app-server host, not the TUI
/// host, because remote fs APIs deserialize and resolve paths on the server.
pub(crate) type GoalFilePath = String;
impl GoalFileStore for AppServerSession {
async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> {
if !self.uses_remote_workspace() {
return self
.fs_create_directory(local_goal_file_path(&path)?)
.await
.map_err(|err| anyhow::anyhow!("{err}"));
}
let _: FsCreateDirectoryResponse = self
.request_json_rpc_typed(
"fs/createDirectory",
json!({
"path": path.as_str(),
"recursive": true,
}),
)
.await
.map_err(|err| anyhow::anyhow!("{err}"))?;
let _: FsCreateDirectoryResponse = request_goal_fs(
self,
"fs/createDirectory",
ClientRequest::FsCreateDirectory {
request_id: goal_request_id(),
params: FsCreateDirectoryParams {
path: local_goal_file_path(&path)?,
recursive: Some(true),
},
},
json!({
"path": path,
"recursive": true,
}),
)
.await?;
Ok(())
}
async fn write_file(&mut self, path: GoalFilePath, bytes: Vec<u8>) -> Result<()> {
if !self.uses_remote_workspace() {
return self
.fs_write_file(local_goal_file_path(&path)?, bytes)
.await
.map_err(|err| anyhow::anyhow!("{err}"));
}
let _: FsWriteFileResponse = self
.request_json_rpc_typed(
"fs/writeFile",
json!({
"path": path.as_str(),
"dataBase64": STANDARD.encode(bytes),
}),
)
.await
.map_err(|err| anyhow::anyhow!("{err}"))?;
let data_base64 = STANDARD.encode(bytes);
let _: FsWriteFileResponse = request_goal_fs(
self,
"fs/writeFile",
ClientRequest::FsWriteFile {
request_id: goal_request_id(),
params: FsWriteFileParams {
path: local_goal_file_path(&path)?,
data_base64: data_base64.clone(),
},
},
json!({
"path": path,
"dataBase64": data_base64,
}),
)
.await?;
Ok(())
}
async fn read_file(&mut self, path: GoalFilePath) -> Result<Vec<u8>> {
if !self.uses_remote_workspace() {
return self
.fs_read_file(local_goal_file_path(&path)?)
.await
.map_err(|err| anyhow::anyhow!("{err}"));
}
let response: FsReadFileResponse = self
.request_json_rpc_typed("fs/readFile", json!({ "path": path.as_str() }))
.await
.map_err(|err| anyhow::anyhow!("{err}"))?;
let response: FsReadFileResponse = request_goal_fs(
self,
"fs/readFile",
ClientRequest::FsReadFile {
request_id: goal_request_id(),
params: FsReadFileParams {
path: local_goal_file_path(&path)?,
},
},
json!({ "path": path }),
)
.await?;
STANDARD
.decode(response.data_base64)
.context("fs/readFile returned invalid base64 data")
}
}
async fn request_goal_fs<T>(
app_server: &AppServerSession,
method: &str,
local_request: ClientRequest,
remote_params: serde_json::Value,
) -> Result<T>
where
T: DeserializeOwned,
{
if app_server.uses_remote_workspace() {
return remote_json_rpc_typed(app_server, method, remote_params).await;
}
app_server
.request_handle()
.request_typed(local_request)
.await
.with_context(|| format!("{method} failed in TUI"))
}
async fn remote_json_rpc_typed<T>(
app_server: &AppServerSession,
method: &str,
params: serde_json::Value,
) -> Result<T>
where
T: DeserializeOwned,
{
let AppServerRequestHandle::Remote(handle) = app_server.request_handle() else {
bail!("raw JSON-RPC requests are only supported by the remote app-server client");
};
let response = handle
.request_json_rpc(JSONRPCRequest {
id: goal_request_id(),
method: method.to_string(),
params: Some(params),
trace: None,
})
.await
.with_context(|| format!("{method} failed in TUI"))?;
let result =
response.map_err(|source| anyhow::anyhow!("{method} failed in TUI: {}", source.message))?;
serde_json::from_value(result).with_context(|| format!("{method} returned invalid data"))
}
fn goal_request_id() -> RequestId {
RequestId::String(format!("goal-files-{}", Uuid::new_v4()))
}
pub(crate) fn codex_home_for_app_server(
app_server: &AppServerSession,
local_codex_home: &AbsolutePathBuf,
) -> Option<GoalFilePath> {
if app_server.uses_remote_workspace() {
app_server
.remote_codex_home()
.map(|path| GoalFilePath::from_remote(path, app_server.remote_platform_family()))
app_server.remote_codex_home().map(str::to_string)
} else {
Some(GoalFilePath::from_local(local_codex_home))
Some(local_codex_home.display().to_string())
}
}
fn local_goal_file_path(path: &GoalFilePath) -> Result<AbsolutePathBuf> {
AbsolutePathBuf::from_absolute_path_checked(path.as_str())
.with_context(|| format!("invalid local goal file path {}", path.display()))
AbsolutePathBuf::from_absolute_path_checked(path)
.with_context(|| format!("invalid local goal file path {path}"))
}
fn join_goal_path(path: &str, segment: impl AsRef<str>) -> GoalFilePath {
let separator = if is_windows_absolute_path(path) {
'\\'
} else {
'/'
};
let mut path = path.trim_end_matches(['/', '\\']).to_string();
if !path.ends_with(separator) {
path.push(separator);
}
path.push_str(segment.as_ref());
path
}
fn managed_goal_file_path(raw: &str) -> Option<GoalFilePath> {
if !is_windows_absolute_path(raw) && !raw.starts_with('/') {
return None;
}
let normalized = raw.replace('\\', "/");
let mut parts = normalized.rsplit('/').filter(|part| !part.is_empty());
let file_name = parts.next()?;
let attachment_id = parts.next()?;
let attachment_dir = parts.next()?;
if file_name == GOAL_FILE_NAME
&& attachment_dir == GOAL_ATTACHMENT_DIR
&& Uuid::parse_str(attachment_id).is_ok()
{
Some(raw.to_string())
} else {
None
}
}
pub(crate) async fn materialize_goal_draft(
@@ -233,55 +240,39 @@ pub(crate) async fn materialize_goal_draft(
bail!("Goal objective must not be empty.");
}
let text_elements = draft.text_elements;
let (validation_objective, _) = ChatComposer::expand_pending_pastes(
&objective,
text_elements.clone(),
&draft.pending_pastes,
);
if validation_objective.trim().is_empty() {
bail!("Goal objective must not be empty.");
}
let mut output_dir = None;
let mut materialized_pastes = Vec::new();
let mut replacements = Vec::new();
for (idx, (placeholder, text)) in draft.pending_pastes.iter().enumerate() {
let path = ensure_output_dir(store, codex_home, &mut output_dir)
.await?
.join(format!("pasted-text-{}.txt", idx + 1));
let path = join_goal_path(
&ensure_output_dir(store, codex_home, &mut output_dir).await?,
format!("pasted-text-{}.txt", idx + 1),
);
write_file(store, path.clone(), text.as_bytes().to_vec()).await?;
if !placeholder.is_empty() {
materialized_pastes.push((
placeholder.clone(),
format!("pasted text file: {}", path.display()),
));
replacements.push((placeholder.clone(), format!("pasted text file: {path}")));
}
}
let (expanded_objective, text_elements) =
ChatComposer::expand_pending_pastes(&objective, text_elements, &materialized_pastes);
objective = expanded_objective;
let mut image_lines = Vec::new();
let mut materialized_images = Vec::new();
for (idx, image) in draft.local_images.iter().enumerate() {
let extension = image_extension(&image.path);
let path = ensure_output_dir(store, codex_home, &mut output_dir)
.await?
.join(format!("image-{}.{}", idx + 1, extension));
let path = join_goal_path(
&ensure_output_dir(store, codex_home, &mut output_dir).await?,
format!("image-{}.{}", idx + 1, extension),
);
let bytes = fs::read(&image.path)
.with_context(|| format!("Could not read goal image {}", image.path.display()))?;
write_file(store, path.clone(), bytes).await?;
if image.placeholder.is_empty() {
image_lines.push(format!("- [Image #{}]: {}", idx + 1, path.display()));
image_lines.push(format!("- [Image #{}]: {path}", idx + 1));
} else {
materialized_images.push((
image.placeholder.clone(),
format!("image file: {}", path.display()),
));
replacements.push((image.placeholder.clone(), format!("image file: {path}")));
}
}
let (expanded_objective, _) =
ChatComposer::expand_pending_pastes(&objective, text_elements, &materialized_images);
ChatComposer::expand_pending_pastes(&objective, text_elements, &replacements);
objective = expanded_objective.trim().to_string();
append_section(&mut objective, "Referenced image files:", image_lines);
@@ -296,9 +287,10 @@ pub(crate) async fn materialize_goal_draft(
);
if objective.chars().count() > MAX_THREAD_GOAL_OBJECTIVE_CHARS {
let path = ensure_output_dir(store, codex_home, &mut output_dir)
.await?
.join(GOAL_FILE_NAME);
let path = join_goal_path(
&ensure_output_dir(store, codex_home, &mut output_dir).await?,
GOAL_FILE_NAME,
);
write_file(store, path.clone(), objective.as_bytes().to_vec()).await?;
objective = objective_file_reference(&path)?;
}
@@ -316,9 +308,9 @@ pub(crate) async fn objective_text_for_edit(
let bytes = store
.read_file(path.clone())
.await
.with_context(|| format!("Could not read goal objective file {}", path.display()))?;
.with_context(|| format!("Could not read goal objective file {path}"))?;
String::from_utf8(bytes)
.with_context(|| format!("Goal objective file {} is not valid UTF-8", path.display()))
.with_context(|| format!("Goal objective file {path} is not valid UTF-8"))
}
pub(crate) fn objective_file_path(objective: &str) -> Option<GoalFilePath> {
@@ -328,7 +320,7 @@ pub(crate) fn objective_file_path(objective: &str) -> Option<GoalFilePath> {
.strip_prefix(GOAL_FILE_PREFIX)
.map(str::trim)
.filter(|path| !path.is_empty())
.and_then(GoalFilePath::from_managed_reference)?;
.and_then(managed_goal_file_path)?;
if lines.next() != Some(GOAL_FILE_INSTRUCTION) {
return None;
}
@@ -337,10 +329,7 @@ pub(crate) fn objective_file_path(objective: &str) -> Option<GoalFilePath> {
}
pub(crate) fn objective_file_reference(path: &GoalFilePath) -> Result<String> {
let reference = format!(
"{GOAL_FILE_PREFIX}{}\n{GOAL_FILE_INSTRUCTION}",
path.display()
);
let reference = format!("{GOAL_FILE_PREFIX}{path}\n{GOAL_FILE_INSTRUCTION}");
let actual_chars = reference.chars().count();
if actual_chars > MAX_THREAD_GOAL_OBJECTIVE_CHARS {
bail!(
@@ -360,18 +349,14 @@ async fn ensure_output_dir(
}
let codex_home = codex_home
.context("App server did not report $CODEX_HOME; cannot materialize goal files")?;
let path = codex_home
.join(GOAL_ATTACHMENT_DIR)
.join(Uuid::new_v4().to_string());
let path = join_goal_path(
&join_goal_path(codex_home, GOAL_ATTACHMENT_DIR),
Uuid::new_v4().to_string(),
);
store
.create_directory(path.clone())
.await
.with_context(|| {
format!(
"Could not create goal attachment directory {}",
path.display()
)
})?;
.with_context(|| format!("Could not create goal attachment directory {path}"))?;
*output_dir = Some(path.clone());
Ok(path)
}
@@ -384,7 +369,7 @@ async fn write_file(
store
.write_file(path.clone(), bytes)
.await
.with_context(|| format!("Could not write goal file {}", path.display()))
.with_context(|| format!("Could not write goal file {path}"))
}
fn is_windows_absolute_path(path: &str) -> bool {
@@ -409,11 +394,18 @@ fn append_section(objective: &mut String, heading: &str, lines: Vec<String>) {
objective.push_str(&lines.join("\n"));
}
fn image_extension(path: &Path) -> &str {
fn image_extension(path: &Path) -> String {
path.extension()
.and_then(|extension| extension.to_str())
.map(|extension| {
extension
.chars()
.filter(char::is_ascii_alphanumeric)
.take(8)
.collect::<String>()
})
.filter(|extension| !extension.is_empty())
.unwrap_or("png")
.unwrap_or_else(|| "png".to_string())
}
#[cfg(test)]

187
codex-rs/tui/src/goal_files_tests.rs
View File

@@ -3,40 +3,19 @@ use super::*;
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
use codex_protocol::user_input::TextElement;
use pretty_assertions::assert_eq;
use std::path::Path;
struct LocalStore;
impl GoalFileStore for LocalStore {
async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> {
fs::create_dir_all(path.as_str())?;
Ok(())
}
async fn write_file(&mut self, path: GoalFilePath, bytes: Vec<u8>) -> Result<()> {
fs::write(path.as_str(), bytes)?;
Ok(())
}
async fn read_file(&mut self, path: GoalFilePath) -> Result<Vec<u8>> {
Ok(fs::read(path.as_str())?)
}
}
#[derive(Default)]
struct RecordingStore {
created_dirs: Vec<String>,
writes: Vec<(String, Vec<u8>)>,
}
impl GoalFileStore for RecordingStore {
async fn create_directory(&mut self, path: GoalFilePath) -> Result<()> {
self.created_dirs.push(path.as_str().to_string());
async fn create_directory(&mut self, _path: GoalFilePath) -> Result<()> {
Ok(())
}
async fn write_file(&mut self, path: GoalFilePath, bytes: Vec<u8>) -> Result<()> {
self.writes.push((path.as_str().to_string(), bytes));
self.writes.push((path, bytes));
Ok(())
}
@@ -45,71 +24,40 @@ impl GoalFileStore for RecordingStore {
.iter()
.find(|(write_path, _)| write_path == path.as_str())
.map(|(_, bytes)| bytes.clone())
.ok_or_else(|| anyhow::anyhow!("missing recording for {}", path.display()))
.ok_or_else(|| anyhow::anyhow!("missing recording for {path}"))
}
}
#[tokio::test]
async fn materializes_and_reads_oversized_objective_through_store() {
async fn materializes_oversized_objective_with_remote_windows_path() {
let temp_dir = tempfile::tempdir().expect("tempdir");
let codex_home = local_goal_home(temp_dir.path());
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
let mut store = LocalStore;
let image_path = temp_dir.path().join("local-image.png");
fs::write(&image_path, b"png bytes").expect("write image");
let paste_placeholder = "[Pasted Content 5 chars]";
let image_placeholder = "[Image #1]";
let objective = format!(
"Use {paste_placeholder} and {image_placeholder}. {}",
"x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1)
);
let text_elements = [paste_placeholder, image_placeholder]
.into_iter()
.map(|placeholder| {
let start = objective.find(placeholder).expect("placeholder");
TextElement::new(
(start..start + placeholder.len()).into(),
Some(placeholder.to_string()),
)
})
.collect();
let codex_home = r"C:\Users\codex\.codex".to_string();
let mut store = RecordingStore::default();
let reference = materialize_goal_draft(
&mut store,
Some(&codex_home),
GoalDraft {
objective: objective.clone(),
..Default::default()
},
)
.await
.expect("materialize goal draft");
let path = objective_file_path(&reference).expect("goal file path");
assert_eq!(
fs::read_to_string(path.as_str()).expect("read file"),
objective
);
let edit_text = objective_text_for_edit(&mut store, &reference)
.await
.expect("read objective text");
assert_eq!(edit_text, objective);
}
#[tokio::test]
async fn materializes_paste_and_image_through_store() {
let temp_dir = tempfile::tempdir().expect("tempdir");
let codex_home = local_goal_home(temp_dir.path());
let image_path = temp_dir.path().join("local-image.png");
fs::write(&image_path, b"png bytes").expect("write image");
let mut store = LocalStore;
let objective = "Use [Pasted Content 5 chars] and [Image #1]".to_string();
let paste_placeholder = "[Pasted Content 5 chars]";
let image_placeholder = "[Image #1]";
let paste_start = objective
.find(paste_placeholder)
.expect("paste placeholder");
let image_start = objective
.find(image_placeholder)
.expect("image placeholder");
let objective = materialize_goal_draft(
&mut store,
Some(&codex_home),
GoalDraft {
objective,
text_elements: vec![
TextElement::new(
(paste_start..paste_start + paste_placeholder.len()).into(),
Some(paste_placeholder.to_string()),
),
TextElement::new(
(image_start..image_start + image_placeholder.len()).into(),
Some(image_placeholder.to_string()),
),
],
text_elements,
pending_pastes: vec![(paste_placeholder.to_string(), "hello".to_string())],
local_images: vec![LocalImageAttachment {
placeholder: image_placeholder.to_string(),
@@ -121,46 +69,24 @@ async fn materializes_paste_and_image_through_store() {
.await
.expect("materialize goal draft");
let paste_path = path_after(&objective, "pasted text file: ");
let image_path = path_after(&objective, "image file: ");
assert_eq!(fs::read_to_string(paste_path).expect("read paste"), "hello");
assert_eq!(fs::read(image_path).expect("read image"), b"png bytes");
}
#[tokio::test]
async fn materializes_oversized_objective_with_windows_remote_path() {
let objective = "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1);
let codex_home = GoalFilePath::from_remote(r"C:\Users\codex\.codex", Some("windows"));
let mut store = RecordingStore::default();
let reference = materialize_goal_draft(
&mut store,
Some(&codex_home),
GoalDraft {
objective: objective.clone(),
..Default::default()
},
)
.await
.expect("materialize goal draft");
assert_eq!(store.created_dirs.len(), 1);
assert_eq!(store.writes.len(), 1);
let (path, bytes) = &store.writes[0];
assert!(path.starts_with(r"C:\Users\codex\.codex\attachments\"));
assert!(path.ends_with(r"\goal-objective.md"));
assert_eq!(bytes, objective.as_bytes());
assert_eq!(
objective_file_path(&reference)
.expect("goal file path")
.as_str(),
path
let path = objective_file_path(&reference).expect("goal file path");
assert!(
path.as_str()
.starts_with(r"C:\Users\codex\.codex\attachments\")
);
assert!(path.as_str().ends_with(r"\goal-objective.md"));
let edit_text = objective_text_for_edit(&mut store, &reference)
.await
.expect("read objective text");
assert!(edit_text.contains(r"pasted text file: C:\Users\codex\.codex\attachments\"));
assert!(edit_text.contains(r"image file: C:\Users\codex\.codex\attachments\"));
assert!(store.writes.iter().any(|(_, bytes)| bytes == b"hello"));
assert!(store.writes.iter().any(|(_, bytes)| bytes == b"png bytes"));
}
#[tokio::test]
async fn plain_objective_does_not_need_codex_home() {
let mut store = LocalStore;
let mut store = RecordingStore::default();
let objective = materialize_goal_draft(
&mut store,
@@ -175,40 +101,3 @@ async fn plain_objective_does_not_need_codex_home() {
assert_eq!(objective, "read src/lib.rs");
}
#[tokio::test]
async fn oversized_objective_requires_codex_home() {
let mut store = LocalStore;
let err = materialize_goal_draft(
&mut store,
/*codex_home*/ None,
GoalDraft {
objective: "x".repeat(MAX_THREAD_GOAL_OBJECTIVE_CHARS + 1),
..Default::default()
},
)
.await
.expect_err("oversized objective should require codex home");
assert!(
err.to_string().contains("$CODEX_HOME"),
"expected codex home error, got {err:#}"
);
}
fn path_after(text: &str, prefix: &str) -> String {
let path = text
.split_once(prefix)
.unwrap_or_else(|| panic!("expected {prefix:?} in {text:?}"))
.1
.split_whitespace()
.next()
.expect("path");
path.to_string()
}
fn local_goal_home(path: &Path) -> GoalFilePath {
let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute codex home");
GoalFilePath::from_local(&path)
}