mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Separate TUI preferences from server configuration (#42202)
## Why TUI preferences and persistence paths belong to the local client, while thread configuration and account requirements may come from the active app server. Keeping both in the same resolved `Config` can replace live local preferences when a thread is resumed, forked, reconnected, or switched. ## What changed - Add `LocalSettings` as the TUI-owned source for UI, history, notice, and local persistence settings, and preserve it across session lifecycle transitions. - Write preference changes to the selected user config file and reload local settings from disk when changing roots. - Use the app server's account response for authentication-dependent onboarding and status UI instead of the local model-provider configuration. - Retain platform family and OS metadata reported by remote app servers. ## Testing - Cover local setting defaults, overrides, persistence destinations, reloads, and preservation across widget replacement and root switching. - Cover remote platform metadata and server-controlled authentication UI. GitOrigin-RevId: dbc2965fb1d6cbecb3f973f6133f3dcd4467b753
This commit is contained in:
@@ -677,6 +677,23 @@ impl AppServerRequestHandle {
|
||||
}
|
||||
|
||||
impl AppServerClient {
|
||||
/// App-server platform family, which can differ from the executor's platform.
|
||||
/// Older remote servers may omit this metadata.
|
||||
pub fn platform_family(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::InProcess(_) => Some(std::env::consts::FAMILY),
|
||||
Self::Remote(client) => client.platform_family(),
|
||||
}
|
||||
}
|
||||
|
||||
/// App-server operating system as reported at initialization, including unknown values.
|
||||
pub fn platform_os(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::InProcess(_) => Some(std::env::consts::OS),
|
||||
Self::Remote(client) => client.platform_os(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn codex_home(&self, local_codex_home: &AbsolutePathBuf) -> Option<AppServerPath> {
|
||||
match self {
|
||||
Self::InProcess(_) => Some(AppServerPath::from_app_server(
|
||||
@@ -930,6 +947,22 @@ mod tests {
|
||||
async fn expect_remote_initialize<S>(websocket: &mut tokio_tungstenite::WebSocketStream<S>)
|
||||
where
|
||||
S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin,
|
||||
{
|
||||
expect_remote_initialize_with_metadata(
|
||||
websocket,
|
||||
serde_json::json!({
|
||||
"userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust",
|
||||
"codexHome": "/server/.codex",
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn expect_remote_initialize_with_metadata<S>(
|
||||
websocket: &mut tokio_tungstenite::WebSocketStream<S>,
|
||||
metadata: serde_json::Value,
|
||||
) where
|
||||
S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin,
|
||||
{
|
||||
let JSONRPCMessage::Request(request) = read_websocket_message(websocket).await else {
|
||||
panic!("expected initialize request");
|
||||
@@ -939,10 +972,7 @@ mod tests {
|
||||
websocket,
|
||||
JSONRPCMessage::Response(JSONRPCResponse {
|
||||
id: request.id,
|
||||
result: serde_json::json!({
|
||||
"userAgent": "codex_cli_rs/9.8.7-test (Test OS; x86_64) rust",
|
||||
"codexHome": "/server/.codex",
|
||||
}),
|
||||
result: metadata,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
@@ -1079,7 +1109,15 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn typed_request_roundtrip_works() {
|
||||
let client = start_test_client(SessionSource::Exec).await;
|
||||
let TestClient {
|
||||
_codex_home,
|
||||
client,
|
||||
} = start_test_client(SessionSource::Exec).await;
|
||||
let client = AppServerClient::InProcess(client);
|
||||
assert_eq!(
|
||||
(client.platform_family(), client.platform_os()),
|
||||
(Some(std::env::consts::FAMILY), Some(std::env::consts::OS))
|
||||
);
|
||||
let _response: ConfigRequirementsReadResponse = client
|
||||
.request_typed(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(1),
|
||||
@@ -1255,6 +1293,41 @@ mod tests {
|
||||
client.shutdown().await.expect("shutdown should complete");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_platform_metadata_preserves_reported_and_missing_values() {
|
||||
for (family, os) in [
|
||||
(Some("windows"), Some("windows")),
|
||||
(Some("unix"), Some("linux")),
|
||||
(Some("future-family"), Some("future-os")),
|
||||
(Some("unix"), None),
|
||||
(None, Some("linux")),
|
||||
(None, None),
|
||||
] {
|
||||
let websocket_url = start_test_remote_server(move |mut websocket| async move {
|
||||
let mut metadata = serde_json::json!({});
|
||||
if let Some(family) = family {
|
||||
metadata["platformFamily"] = family.into();
|
||||
}
|
||||
if let Some(os) = os {
|
||||
metadata["platformOs"] = os.into();
|
||||
}
|
||||
expect_remote_initialize_with_metadata(&mut websocket, metadata).await;
|
||||
websocket.close(None).await.expect("close should succeed");
|
||||
})
|
||||
.await;
|
||||
let client = AppServerClient::Remote(
|
||||
RemoteAppServerClient::connect(test_remote_connect_args(websocket_url))
|
||||
.await
|
||||
.expect("remote client should connect"),
|
||||
);
|
||||
assert_eq!(
|
||||
(client.platform_family(), client.platform_os()),
|
||||
(family, os)
|
||||
);
|
||||
client.shutdown().await.expect("shutdown should complete");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_typed_request_roundtrip_works() {
|
||||
let websocket_url = start_test_remote_server(|mut websocket| async move {
|
||||
|
||||
@@ -148,12 +148,19 @@ enum RemoteClientCommand {
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct RemoteServerMetadata {
|
||||
server_version: Option<String>,
|
||||
codex_home: Option<String>,
|
||||
platform_family: Option<String>,
|
||||
platform_os: Option<String>,
|
||||
}
|
||||
|
||||
pub struct RemoteAppServerClient {
|
||||
command_tx: mpsc::Sender<RemoteClientCommand>,
|
||||
event_rx: mpsc::UnboundedReceiver<AppServerEvent>,
|
||||
pending_events: VecDeque<AppServerEvent>,
|
||||
server_version: Option<String>,
|
||||
codex_home: Option<String>,
|
||||
metadata: RemoteServerMetadata,
|
||||
worker_handle: tokio::task::JoinHandle<()>,
|
||||
}
|
||||
|
||||
@@ -185,11 +192,19 @@ impl RemoteAppServerClient {
|
||||
}
|
||||
|
||||
pub fn server_version(&self) -> Option<&str> {
|
||||
self.server_version.as_deref()
|
||||
self.metadata.server_version.as_deref()
|
||||
}
|
||||
|
||||
pub fn codex_home(&self) -> Option<&str> {
|
||||
self.codex_home.as_deref()
|
||||
self.metadata.codex_home.as_deref()
|
||||
}
|
||||
|
||||
pub fn platform_family(&self) -> Option<&str> {
|
||||
self.metadata.platform_family.as_deref()
|
||||
}
|
||||
|
||||
pub fn platform_os(&self) -> Option<&str> {
|
||||
self.metadata.platform_os.as_deref()
|
||||
}
|
||||
|
||||
async fn connect_with_stream<S>(
|
||||
@@ -202,7 +217,7 @@ impl RemoteAppServerClient {
|
||||
S: AsyncRead + AsyncWrite + Unpin + Send + 'static,
|
||||
{
|
||||
let mut stream = stream;
|
||||
let (pending_events, server_version, codex_home) = initialize_remote_connection(
|
||||
let (pending_events, metadata) = initialize_remote_connection(
|
||||
&mut stream,
|
||||
&endpoint,
|
||||
initialize_params,
|
||||
@@ -478,8 +493,7 @@ impl RemoteAppServerClient {
|
||||
command_tx,
|
||||
event_rx,
|
||||
pending_events: pending_events.into(),
|
||||
server_version,
|
||||
codex_home,
|
||||
metadata,
|
||||
worker_handle,
|
||||
})
|
||||
}
|
||||
@@ -604,8 +618,7 @@ impl RemoteAppServerClient {
|
||||
command_tx,
|
||||
event_rx,
|
||||
pending_events: _pending_events,
|
||||
server_version: _server_version,
|
||||
codex_home: _codex_home,
|
||||
metadata: _,
|
||||
worker_handle,
|
||||
} = self;
|
||||
let mut worker_handle = worker_handle;
|
||||
@@ -800,14 +813,13 @@ async fn initialize_remote_connection<S>(
|
||||
endpoint: &str,
|
||||
params: InitializeParams,
|
||||
initialize_timeout: Duration,
|
||||
) -> IoResult<(Vec<AppServerEvent>, Option<String>, Option<String>)>
|
||||
) -> IoResult<(Vec<AppServerEvent>, RemoteServerMetadata)>
|
||||
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 codex_home = None;
|
||||
let mut metadata = RemoteServerMetadata::default();
|
||||
write_jsonrpc_message(
|
||||
stream,
|
||||
JSONRPCMessage::Request(jsonrpc_request_from_client_request(
|
||||
@@ -831,7 +843,7 @@ where
|
||||
})?;
|
||||
match message {
|
||||
JSONRPCMessage::Response(response) if response.id == initialize_request_id => {
|
||||
server_version = response
|
||||
metadata.server_version = response
|
||||
.result
|
||||
.get("userAgent")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
@@ -839,12 +851,16 @@ where
|
||||
let (_, rest) = user_agent.split_once('/')?;
|
||||
rest.split_whitespace().next().map(str::to_string)
|
||||
});
|
||||
codex_home = response
|
||||
metadata.codex_home = response
|
||||
.result
|
||||
.get("codexHome")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.filter(|codex_home| !codex_home.is_empty())
|
||||
.map(str::to_string);
|
||||
metadata.platform_family = response.result.get("platformFamily")
|
||||
.and_then(serde_json::Value::as_str).map(str::to_string);
|
||||
metadata.platform_os = response.result.get("platformOs")
|
||||
.and_then(serde_json::Value::as_str).map(str::to_string);
|
||||
break Ok(());
|
||||
}
|
||||
JSONRPCMessage::Error(error) if error.id == initialize_request_id => {
|
||||
@@ -937,7 +953,7 @@ where
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok((pending_events, server_version, codex_home))
|
||||
Ok((pending_events, metadata))
|
||||
}
|
||||
|
||||
fn app_server_event_from_notification(notification: JSONRPCNotification) -> Option<AppServerEvent> {
|
||||
@@ -1027,8 +1043,7 @@ mod tests {
|
||||
command_tx,
|
||||
event_rx,
|
||||
pending_events: VecDeque::new(),
|
||||
server_version: None,
|
||||
codex_home: None,
|
||||
metadata: RemoteServerMetadata::default(),
|
||||
worker_handle,
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user