mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
Fix remote goal file materialization
This commit is contained in:
@@ -53,6 +53,7 @@ pub use codex_exec_server::EnvironmentManager;
|
||||
pub use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_feedback::CodexFeedback;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::de::DeserializeOwned;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::oneshot;
|
||||
@@ -899,6 +900,13 @@ impl AppServerClient {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn remote_codex_home(&self) -> Option<&AbsolutePathBuf> {
|
||||
match self {
|
||||
Self::InProcess(_) => None,
|
||||
Self::Remote(client) => client.codex_home(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn request_handle(&self) -> AppServerRequestHandle {
|
||||
match self {
|
||||
Self::InProcess(client) => AppServerRequestHandle::InProcess(client.request_handle()),
|
||||
@@ -1104,6 +1112,9 @@ mod tests {
|
||||
id: request.id,
|
||||
result: serde_json::json!({
|
||||
"userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust",
|
||||
"codexHome": test_remote_codex_home().display().to_string(),
|
||||
"platformFamily": "unix",
|
||||
"platformOs": "linux",
|
||||
}),
|
||||
}),
|
||||
)
|
||||
@@ -1116,6 +1127,11 @@ mod tests {
|
||||
assert_eq!(notification.method, "initialized");
|
||||
}
|
||||
|
||||
fn test_remote_codex_home() -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::from_absolute_path(std::env::temp_dir().join("codex-remote-home"))
|
||||
.expect("test remote codex home should be absolute")
|
||||
}
|
||||
|
||||
async fn read_websocket_message<S>(
|
||||
websocket: &mut tokio_tungstenite::WebSocketStream<S>,
|
||||
) -> JSONRPCMessage
|
||||
@@ -1440,6 +1456,7 @@ mod tests {
|
||||
.expect("remote client should connect");
|
||||
|
||||
assert_eq!(client.server_version(), Some("9.8.7-test"));
|
||||
assert_eq!(client.codex_home(), Some(&test_remote_codex_home()));
|
||||
let response: GetAccountResponse = client
|
||||
.request_typed(ClientRequest::GetAccount {
|
||||
request_id: RequestId::Integer(1),
|
||||
|
||||
@@ -151,9 +151,16 @@ pub struct RemoteAppServerClient {
|
||||
event_rx: mpsc::UnboundedReceiver<AppServerEvent>,
|
||||
pending_events: VecDeque<AppServerEvent>,
|
||||
server_version: Option<String>,
|
||||
codex_home: Option<AbsolutePathBuf>,
|
||||
worker_handle: tokio::task::JoinHandle<()>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
struct RemoteInitializeInfo {
|
||||
server_version: Option<String>,
|
||||
codex_home: Option<AbsolutePathBuf>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct RemoteAppServerRequestHandle {
|
||||
command_tx: mpsc::Sender<RemoteClientCommand>,
|
||||
@@ -185,6 +192,10 @@ impl RemoteAppServerClient {
|
||||
self.server_version.as_deref()
|
||||
}
|
||||
|
||||
pub fn codex_home(&self) -> Option<&AbsolutePathBuf> {
|
||||
self.codex_home.as_ref()
|
||||
}
|
||||
|
||||
async fn connect_with_stream<S>(
|
||||
channel_capacity: usize,
|
||||
endpoint: String,
|
||||
@@ -195,7 +206,7 @@ impl RemoteAppServerClient {
|
||||
S: AsyncRead + AsyncWrite + Unpin + Send + 'static,
|
||||
{
|
||||
let mut stream = stream;
|
||||
let (pending_events, server_version) = initialize_remote_connection(
|
||||
let (pending_events, initialize_info) = initialize_remote_connection(
|
||||
&mut stream,
|
||||
&endpoint,
|
||||
initialize_params,
|
||||
@@ -471,7 +482,8 @@ impl RemoteAppServerClient {
|
||||
command_tx,
|
||||
event_rx,
|
||||
pending_events: pending_events.into(),
|
||||
server_version,
|
||||
server_version: initialize_info.server_version,
|
||||
codex_home: initialize_info.codex_home,
|
||||
worker_handle,
|
||||
})
|
||||
}
|
||||
@@ -613,6 +625,7 @@ impl RemoteAppServerClient {
|
||||
event_rx,
|
||||
pending_events: _pending_events,
|
||||
server_version: _server_version,
|
||||
codex_home: _codex_home,
|
||||
worker_handle,
|
||||
} = self;
|
||||
let mut worker_handle = worker_handle;
|
||||
@@ -800,13 +813,13 @@ async fn initialize_remote_connection<S>(
|
||||
endpoint: &str,
|
||||
params: InitializeParams,
|
||||
initialize_timeout: Duration,
|
||||
) -> IoResult<(Vec<AppServerEvent>, Option<String>)>
|
||||
) -> IoResult<(Vec<AppServerEvent>, RemoteInitializeInfo)>
|
||||
where
|
||||
S: AsyncRead + AsyncWrite + Unpin,
|
||||
{
|
||||
let initialize_request_id = RequestId::String("initialize".to_string());
|
||||
let mut pending_events = Vec::new();
|
||||
let mut server_version = None;
|
||||
let mut initialize_info = RemoteInitializeInfo::default();
|
||||
write_jsonrpc_message(
|
||||
stream,
|
||||
JSONRPCMessage::Request(jsonrpc_request_from_client_request(
|
||||
@@ -830,7 +843,7 @@ where
|
||||
})?;
|
||||
match message {
|
||||
JSONRPCMessage::Response(response) if response.id == initialize_request_id => {
|
||||
server_version = response
|
||||
initialize_info.server_version = response
|
||||
.result
|
||||
.get("userAgent")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
@@ -838,6 +851,11 @@ where
|
||||
let (_, rest) = user_agent.split_once('/')?;
|
||||
rest.split_whitespace().next().map(str::to_string)
|
||||
});
|
||||
initialize_info.codex_home = response
|
||||
.result
|
||||
.get("codexHome")
|
||||
.cloned()
|
||||
.and_then(|value| serde_json::from_value(value).ok());
|
||||
break Ok(());
|
||||
}
|
||||
JSONRPCMessage::Error(error) if error.id == initialize_request_id => {
|
||||
@@ -929,7 +947,7 @@ where
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok((pending_events, server_version))
|
||||
Ok((pending_events, initialize_info))
|
||||
}
|
||||
|
||||
fn app_server_event_from_notification(notification: JSONRPCNotification) -> Option<AppServerEvent> {
|
||||
@@ -1024,6 +1042,7 @@ mod tests {
|
||||
event_rx,
|
||||
pending_events: VecDeque::new(),
|
||||
server_version: None,
|
||||
codex_home: None,
|
||||
worker_handle,
|
||||
};
|
||||
|
||||
|
||||
@@ -131,8 +131,9 @@ impl App {
|
||||
draft: goal_files::GoalDraft,
|
||||
mode: ThreadGoalSetMode,
|
||||
) {
|
||||
let codex_home = app_server.goal_files_codex_home(&self.config.codex_home);
|
||||
let result =
|
||||
goal_files::materialize_goal_draft(app_server, &self.config.codex_home, draft).await;
|
||||
goal_files::materialize_goal_draft(app_server, codex_home.as_ref(), draft).await;
|
||||
let objective = match result {
|
||||
Ok(objective) => objective,
|
||||
Err(err) => {
|
||||
|
||||
@@ -238,6 +238,16 @@ impl AppServerSession {
|
||||
matches!(self.thread_params_mode, ThreadParamsMode::Remote)
|
||||
}
|
||||
|
||||
pub(crate) fn goal_files_codex_home(
|
||||
&self,
|
||||
local_codex_home: &AbsolutePathBuf,
|
||||
) -> Option<AbsolutePathBuf> {
|
||||
match self.thread_params_mode {
|
||||
ThreadParamsMode::Embedded => Some(local_codex_home.clone()),
|
||||
ThreadParamsMode::Remote => self.client.remote_codex_home().cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn server_version(&self) -> Option<&str> {
|
||||
let AppServerClient::Remote(client) = &self.client else {
|
||||
return None;
|
||||
|
||||
@@ -659,6 +659,43 @@ async fn goal_slash_command_uses_plain_text_for_mentions() {
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_slash_command_preserves_selected_file_path_text() {
|
||||
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);
|
||||
chat.bottom_pane
|
||||
.set_composer_text("/goal inspect @read".to_string(), Vec::new(), Vec::new());
|
||||
chat.bottom_pane.on_file_search_result(
|
||||
"read".to_string(),
|
||||
vec![codex_file_search::FileMatch {
|
||||
score: 1,
|
||||
path: PathBuf::from("README.md"),
|
||||
match_type: codex_file_search::MatchType::File,
|
||||
root: test_project_path(),
|
||||
indices: None,
|
||||
}],
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE));
|
||||
submit_current_composer(&mut chat);
|
||||
|
||||
let (actual_thread_id, draft) = loop {
|
||||
match rx.try_recv().expect("expected goal draft event") {
|
||||
AppEvent::SetThreadGoalDraft {
|
||||
thread_id, draft, ..
|
||||
} => break (thread_id, draft),
|
||||
_ => continue,
|
||||
}
|
||||
};
|
||||
assert_eq!(actual_thread_id, thread_id);
|
||||
assert_eq!(draft.objective, "inspect README.md");
|
||||
assert!(draft.pending_pastes.is_empty());
|
||||
assert!(draft.local_images.is_empty());
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_slash_command_emits_attached_images() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
@@ -76,7 +76,7 @@ impl GoalFileStore for AppServerSession {
|
||||
|
||||
pub(crate) async fn materialize_goal_draft(
|
||||
store: &mut impl GoalFileStore,
|
||||
codex_home: &AbsolutePathBuf,
|
||||
codex_home: Option<&AbsolutePathBuf>,
|
||||
draft: GoalDraft,
|
||||
) -> Result<String> {
|
||||
let mut objective = draft.objective;
|
||||
@@ -214,12 +214,14 @@ pub(crate) fn objective_file_reference(path: &Path) -> Result<String> {
|
||||
|
||||
async fn ensure_output_dir(
|
||||
store: &mut impl GoalFileStore,
|
||||
codex_home: &AbsolutePathBuf,
|
||||
codex_home: Option<&AbsolutePathBuf>,
|
||||
output_dir: &mut Option<AbsolutePathBuf>,
|
||||
) -> Result<AbsolutePathBuf> {
|
||||
if let Some(output_dir) = output_dir {
|
||||
return Ok(output_dir.clone());
|
||||
}
|
||||
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());
|
||||
|
||||
@@ -32,7 +32,7 @@ async fn materializes_and_reads_oversized_objective_through_store() {
|
||||
|
||||
let reference = materialize_goal_draft(
|
||||
&mut store,
|
||||
&codex_home,
|
||||
Some(&codex_home),
|
||||
GoalDraft {
|
||||
objective: objective.clone(),
|
||||
..Default::default()
|
||||
@@ -72,7 +72,7 @@ async fn materializes_paste_and_image_through_store() {
|
||||
|
||||
let objective = materialize_goal_draft(
|
||||
&mut store,
|
||||
&codex_home,
|
||||
Some(&codex_home),
|
||||
GoalDraft {
|
||||
objective,
|
||||
text_elements: vec![
|
||||
@@ -118,3 +118,42 @@ fn path_after(text: &str, prefix: &str) -> AbsolutePathBuf {
|
||||
.expect("path");
|
||||
AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute path")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plain_objective_does_not_need_codex_home() {
|
||||
let mut store = LocalStore;
|
||||
|
||||
let objective = materialize_goal_draft(
|
||||
&mut store,
|
||||
/*codex_home*/ None,
|
||||
GoalDraft {
|
||||
objective: "read src/lib.rs".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("materialize plain goal draft");
|
||||
|
||||
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:#}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user