mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(app-server): persist remote-control desired state (#27445)
## Why
Remote-control runtime enablement and persisted enrollment preference
were represented by separate flags. That made startup rehydration, RPC
persistence, and new-enrollment seeding race with one another, and it
did not cleanly distinguish runtime-only CLI or daemon starts from
durable app-server RPC changes.
## What Changed
- Replace the parallel enablement, seed, and rehydration flags with one
transport-owned `RemoteControlDesiredState`.
- Add nullable enrollment-scoped persistence and preserve existing
preferences during enrollment upserts.
- Rehydrate plain startup only after auth and client scope resolve,
without overwriting a concurrent RPC transition.
- Make ordinary `remoteControl/enable` and `remoteControl/disable`
durable while retaining `ephemeral: true` for runtime-only callers.
- Have the daemon explicitly request ephemeral enablement and regenerate
the app-server schemas.
## Verification
- Covered migration and `NULL`/`0`/`1` persistence round trips.
- Covered plain-start rehydration and runtime-only versus durable
enrollment seeding.
- Covered durable enable, durable disable, and ephemeral enable through
app-server RPC.
- Covered the daemon's exact `{ "ephemeral": true }` request payload.
Related issue: N/A (internal remote-control persistence architecture
change).
This commit is contained in:
committed by
GitHub
Unverified
parent
be338ee9a2
commit
d61dfeb23a
@@ -110,10 +110,12 @@ mod transport;
|
||||
pub use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE;
|
||||
pub use crate::error_code::INVALID_PARAMS_ERROR_CODE;
|
||||
pub use crate::transport::AppServerTransport;
|
||||
pub use crate::transport::RemoteControlStartupMode;
|
||||
pub use crate::transport::app_server_control_socket_path;
|
||||
pub use crate::transport::auth::AppServerWebsocketAuthArgs;
|
||||
pub use crate::transport::auth::AppServerWebsocketAuthSettings;
|
||||
pub use crate::transport::auth::WebsocketAuthCliMode;
|
||||
pub use crate::transport::take_remote_control_disabled_env;
|
||||
|
||||
const LOG_FORMAT_ENV_VAR: &str = "LOG_FORMAT";
|
||||
const OTEL_SERVICE_NAME: &str = "codex-app-server";
|
||||
@@ -408,7 +410,7 @@ pub enum PluginStartupTasks {
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct AppServerRuntimeOptions {
|
||||
pub plugin_startup_tasks: PluginStartupTasks,
|
||||
pub remote_control_enabled: bool,
|
||||
pub remote_control_startup_mode: RemoteControlStartupMode,
|
||||
pub install_shutdown_signal_handler: bool,
|
||||
}
|
||||
|
||||
@@ -416,7 +418,7 @@ impl Default for AppServerRuntimeOptions {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
plugin_startup_tasks: PluginStartupTasks::Start,
|
||||
remote_control_enabled: false,
|
||||
remote_control_startup_mode: RemoteControlStartupMode::ResolvePersisted,
|
||||
install_shutdown_signal_handler: true,
|
||||
}
|
||||
}
|
||||
@@ -717,15 +719,21 @@ pub async fn run_main_with_transport_options(
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await;
|
||||
|
||||
let remote_control_requested = runtime_options.remote_control_enabled;
|
||||
let remote_control_enabled = remote_control_requested && state_db.is_some();
|
||||
if remote_control_requested && state_db.is_none() {
|
||||
let remote_control_startup_mode = runtime_options.remote_control_startup_mode;
|
||||
let remote_control_explicitly_requested =
|
||||
remote_control_startup_mode == RemoteControlStartupMode::EnabledEphemeral;
|
||||
let remote_control_enabled = remote_control_explicitly_requested && state_db.is_some();
|
||||
if remote_control_explicitly_requested && state_db.is_none() {
|
||||
error!("remote control disabled because sqlite state db is unavailable");
|
||||
}
|
||||
if transport_accept_handles.is_empty() && !remote_control_enabled {
|
||||
let no_local_transport = transport_accept_handles.is_empty();
|
||||
if no_local_transport
|
||||
&& remote_control_startup_mode != RemoteControlStartupMode::ResolvePersisted
|
||||
&& !remote_control_enabled
|
||||
{
|
||||
return Err(std::io::Error::new(
|
||||
ErrorKind::InvalidInput,
|
||||
if remote_control_requested && state_db.is_none() {
|
||||
if remote_control_explicitly_requested && state_db.is_none() {
|
||||
"no transport configured; remote control disabled because sqlite state db is unavailable"
|
||||
} else {
|
||||
"no transport configured; use --listen or enable remote control"
|
||||
@@ -743,9 +751,31 @@ pub async fn run_main_with_transport_options(
|
||||
transport_event_tx.clone(),
|
||||
transport_shutdown_token.clone(),
|
||||
app_server_client_name_rx,
|
||||
remote_control_enabled,
|
||||
remote_control_startup_mode,
|
||||
)
|
||||
.await?;
|
||||
if no_local_transport
|
||||
&& remote_control_startup_mode == RemoteControlStartupMode::ResolvePersisted
|
||||
{
|
||||
let persisted_enabled = match remote_control_handle
|
||||
.resolve_persisted_preference(/*app_server_client_name*/ None)
|
||||
.await
|
||||
{
|
||||
Ok(persisted_enabled) => persisted_enabled,
|
||||
Err(err) => {
|
||||
warn!("failed to resolve persisted remote control preference: {err}");
|
||||
false
|
||||
}
|
||||
};
|
||||
if !persisted_enabled {
|
||||
transport_shutdown_token.cancel();
|
||||
let _ = remote_control_accept_handle.await;
|
||||
return Err(std::io::Error::new(
|
||||
ErrorKind::InvalidInput,
|
||||
"no transport configured; use --listen or enable remote control",
|
||||
));
|
||||
}
|
||||
}
|
||||
transport_accept_handles.push(remote_control_accept_handle);
|
||||
|
||||
let outbound_handle = tokio::spawn(async move {
|
||||
|
||||
@@ -53,13 +53,14 @@ struct AppServerArgs {
|
||||
#[arg(long = "disable-plugin-startup-tasks-for-tests", hide = true)]
|
||||
disable_plugin_startup_tasks_for_tests: bool,
|
||||
|
||||
/// Enable remote control for this app-server process.
|
||||
/// Enable remote control for this app-server process without changing persistence.
|
||||
#[arg(long = "remote-control", hide = true)]
|
||||
remote_control: bool,
|
||||
}
|
||||
|
||||
fn main() -> anyhow::Result<()> {
|
||||
arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move {
|
||||
let remote_control_disabled = codex_app_server::take_remote_control_disabled_env();
|
||||
arg0_dispatch_or_else(move |arg0_paths: Arg0DispatchPaths| async move {
|
||||
let AppServerArgs {
|
||||
config_overrides,
|
||||
listen,
|
||||
@@ -84,7 +85,12 @@ fn main() -> anyhow::Result<()> {
|
||||
if disable_plugin_startup_tasks_for_tests {
|
||||
runtime_options.plugin_startup_tasks = PluginStartupTasks::Skip;
|
||||
}
|
||||
runtime_options.remote_control_enabled = remote_control;
|
||||
runtime_options.remote_control_startup_mode =
|
||||
match (remote_control, remote_control_disabled) {
|
||||
(true, _) => codex_app_server::RemoteControlStartupMode::EnabledEphemeral,
|
||||
(false, true) => codex_app_server::RemoteControlStartupMode::DisabledEphemeral,
|
||||
(false, false) => codex_app_server::RemoteControlStartupMode::ResolvePersisted,
|
||||
};
|
||||
|
||||
run_main_with_transport_options(
|
||||
arg0_paths,
|
||||
|
||||
@@ -951,13 +951,21 @@ impl MessageProcessor {
|
||||
.experimental_feature_enablement_set(request_id.clone(), params)
|
||||
.await
|
||||
}
|
||||
ClientRequest::RemoteControlEnable { .. } => self
|
||||
ClientRequest::RemoteControlEnable { params, .. } => self
|
||||
.remote_control_processor
|
||||
.enable()
|
||||
.enable(
|
||||
params.is_some_and(|params| params.ephemeral),
|
||||
app_server_client_name.as_deref(),
|
||||
)
|
||||
.await
|
||||
.map(|response| Some(response.into())),
|
||||
ClientRequest::RemoteControlDisable { .. } => self
|
||||
ClientRequest::RemoteControlDisable { params, .. } => self
|
||||
.remote_control_processor
|
||||
.disable()
|
||||
.disable(
|
||||
params.is_some_and(|params| params.ephemeral),
|
||||
app_server_client_name.as_deref(),
|
||||
)
|
||||
.await
|
||||
.map(|response| Some(response.into())),
|
||||
ClientRequest::RemoteControlStatusRead { .. } => self
|
||||
.remote_control_processor
|
||||
|
||||
@@ -28,17 +28,38 @@ impl RemoteControlRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn enable(&self) -> Result<RemoteControlEnableResponse, JSONRPCErrorError> {
|
||||
pub(crate) async fn enable(
|
||||
&self,
|
||||
ephemeral: bool,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> Result<RemoteControlEnableResponse, JSONRPCErrorError> {
|
||||
let handle = self.handle()?;
|
||||
handle
|
||||
.enable()
|
||||
.map(RemoteControlEnableResponse::from)
|
||||
.map_err(map_unavailable)
|
||||
let status = if ephemeral {
|
||||
handle.enable_ephemeral().map_err(map_unavailable)?
|
||||
} else {
|
||||
handle
|
||||
.enable(app_server_client_name)
|
||||
.await
|
||||
.map_err(map_update_error)?
|
||||
};
|
||||
Ok(RemoteControlEnableResponse::from(status))
|
||||
}
|
||||
|
||||
pub(crate) fn disable(&self) -> Result<RemoteControlDisableResponse, JSONRPCErrorError> {
|
||||
pub(crate) async fn disable(
|
||||
&self,
|
||||
ephemeral: bool,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> Result<RemoteControlDisableResponse, JSONRPCErrorError> {
|
||||
let handle = self.handle()?;
|
||||
Ok(RemoteControlDisableResponse::from(handle.disable()))
|
||||
let status = if ephemeral {
|
||||
handle.disable_ephemeral().await
|
||||
} else {
|
||||
handle
|
||||
.disable(app_server_client_name)
|
||||
.await
|
||||
.map_err(map_update_error)?
|
||||
};
|
||||
Ok(RemoteControlDisableResponse::from(status))
|
||||
}
|
||||
|
||||
pub(crate) fn status_read(&self) -> Result<RemoteControlStatusReadResponse, JSONRPCErrorError> {
|
||||
@@ -104,6 +125,14 @@ fn map_unavailable(err: RemoteControlUnavailable) -> JSONRPCErrorError {
|
||||
invalid_request(err.to_string())
|
||||
}
|
||||
|
||||
fn map_update_error(err: io::Error) -> JSONRPCErrorError {
|
||||
if err.kind() == io::ErrorKind::NotFound {
|
||||
invalid_request(err.to_string())
|
||||
} else {
|
||||
internal_error(err.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
fn map_pairing_start_error(err: io::Error) -> JSONRPCErrorError {
|
||||
if err.kind() == io::ErrorKind::InvalidInput {
|
||||
invalid_request(err.to_string())
|
||||
|
||||
@@ -20,6 +20,7 @@ pub(crate) use codex_app_server_transport::OutgoingMessage;
|
||||
pub(crate) use codex_app_server_transport::QueuedOutgoingMessage;
|
||||
pub(crate) use codex_app_server_transport::RemoteControlHandle;
|
||||
pub(crate) use codex_app_server_transport::RemoteControlStartConfig;
|
||||
pub use codex_app_server_transport::RemoteControlStartupMode;
|
||||
pub(crate) use codex_app_server_transport::RemoteControlUnavailable;
|
||||
pub(crate) use codex_app_server_transport::TransportEvent;
|
||||
pub(crate) use codex_app_server_transport::acquire_app_server_startup_lock;
|
||||
@@ -31,6 +32,7 @@ pub(crate) use codex_app_server_transport::start_control_socket_acceptor;
|
||||
pub(crate) use codex_app_server_transport::start_remote_control;
|
||||
pub(crate) use codex_app_server_transport::start_stdio_connection;
|
||||
pub(crate) use codex_app_server_transport::start_websocket_acceptor;
|
||||
pub use codex_app_server_transport::take_remote_control_disabled_env;
|
||||
|
||||
pub(crate) struct ConnectionState {
|
||||
pub(crate) outbound_initialized: Arc<AtomicBool>,
|
||||
|
||||
Reference in New Issue
Block a user