mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
app-server: run initialized rpcs with keyed serialization (#17373)
## Why Initialized app-server RPCs no longer need to bottleneck behind one request processor path. Running them concurrently improves responsiveness, but several request families still mutate shared state or depend on ordered side effects. Those stateful families need an auditable serialization contract so concurrency does not reorder thread, config, auth, command, watcher, MCP, or similar state transitions. This PR keeps that boundary explicit: stateful work is serialized by the smallest useful key, while intentionally read-only or externally concurrent work remains unkeyed. In particular, `thread/list` and `thread/turns/list` explicitly have no serialization because they primarily read append-only rollout storage and should continue to be served concurrently. ## What changed - Adds `ClientRequest::serialization_scope()` in `app-server-protocol` and requires every client request definition to declare its serialization behavior. - Introduces keyed request scopes for thread, thread path, command exec process, fuzzy search session, fs watch, MCP OAuth, and global state buckets such as config, account auth, memory, and device keys. - Routes initialized app-server RPCs through per-key FIFO serialization while allowing unkeyed initialized requests to run concurrently. - Cancels in-flight initialized RPC work when the connection disconnects or the app-server exits so spawned request tasks do not outlive their session. - Adds focused coverage for representative keyed and unkeyed serialization scopes, including explicitly concurrent `thread/turns/list` behavior. ## Validation - Added protocol tests for representative keyed serialization scopes and intentionally unkeyed request families. - Added app-server request serialization tests covering per-key FIFO behavior, concurrent unkeyed execution, disconnect shutdown, and config read-after-write ordering. - Local focused protocol validation after the latest rebase is currently blocked by packageproxy failing to resolve locked `rustls-webpki 0.103.13`; CI is expected to provide the full validation signal.
This commit is contained in:
committed by
GitHub
Unverified
parent
7f7c7c2c07
commit
0700f979ba
@@ -393,6 +393,8 @@ use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::Semaphore;
|
||||
use tokio::sync::SemaphorePermit;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::sync::watch;
|
||||
@@ -524,6 +526,10 @@ pub(crate) struct CodexMessageProcessor {
|
||||
pending_thread_unloads: Arc<Mutex<HashSet<ThreadId>>>,
|
||||
thread_state_manager: ThreadStateManager,
|
||||
thread_watch_manager: ThreadWatchManager,
|
||||
/// Serializes mutations of list membership or fields rendered from list
|
||||
/// results. `thread/list` is intentionally not serialized so it can run
|
||||
/// concurrently against mostly append-only storage.
|
||||
thread_list_state_permit: Arc<Semaphore>,
|
||||
command_exec_manager: CommandExecManager,
|
||||
workspace_settings_cache: Arc<workspace_settings::WorkspaceSettingsCache>,
|
||||
pending_fuzzy_searches: Arc<Mutex<HashMap<String, Arc<AtomicBool>>>>,
|
||||
@@ -549,6 +555,7 @@ struct ListenerTaskContext {
|
||||
pending_thread_unloads: Arc<Mutex<HashSet<ThreadId>>>,
|
||||
analytics_events_client: AnalyticsEventsClient,
|
||||
thread_watch_manager: ThreadWatchManager,
|
||||
thread_list_state_permit: Arc<Semaphore>,
|
||||
fallback_model_provider: String,
|
||||
codex_home: PathBuf,
|
||||
}
|
||||
@@ -790,6 +797,7 @@ impl CodexMessageProcessor {
|
||||
pending_thread_unloads: Arc::new(Mutex::new(HashSet::new())),
|
||||
thread_state_manager: ThreadStateManager::new(),
|
||||
thread_watch_manager: ThreadWatchManager::new_with_outgoing(outgoing),
|
||||
thread_list_state_permit: Arc::new(Semaphore::new(/*permits*/ 1)),
|
||||
command_exec_manager: CommandExecManager::default(),
|
||||
workspace_settings_cache: Arc::new(
|
||||
workspace_settings::WorkspaceSettingsCache::default(),
|
||||
@@ -1326,6 +1334,17 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
async fn acquire_thread_list_state_permit(
|
||||
&self,
|
||||
) -> Result<SemaphorePermit<'_>, JSONRPCErrorError> {
|
||||
self.thread_list_state_permit
|
||||
.acquire()
|
||||
.await
|
||||
.map_err(|err| {
|
||||
internal_error(format!("failed to acquire thread list state permit: {err}"))
|
||||
})
|
||||
}
|
||||
|
||||
async fn login_api_key_common(
|
||||
&self,
|
||||
params: &LoginApiKeyParams,
|
||||
@@ -2421,6 +2440,7 @@ impl CodexMessageProcessor {
|
||||
pending_thread_unloads: Arc::clone(&self.pending_thread_unloads),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
thread_list_state_permit: self.thread_list_state_permit.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.to_path_buf(),
|
||||
};
|
||||
@@ -2871,6 +2891,13 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
|
||||
async fn thread_archive(&self, request_id: ConnectionRequestId, params: ThreadArchiveParams) {
|
||||
let _thread_list_state_permit = match self.acquire_thread_list_state_permit().await {
|
||||
Ok(permit) => permit,
|
||||
Err(error) => {
|
||||
self.outgoing.send_error(request_id, error).await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
let result = self.thread_archive_response(params).await;
|
||||
let archived_thread_ids = result
|
||||
.as_ref()
|
||||
@@ -3077,6 +3104,7 @@ impl CodexMessageProcessor {
|
||||
return Err(invalid_request("thread name must not be empty"));
|
||||
};
|
||||
|
||||
let _thread_list_state_permit = self.acquire_thread_list_state_permit().await?;
|
||||
if let Ok(thread) = self.thread_manager.get_thread(thread_id).await {
|
||||
self.submit_core_op(request_id, thread.as_ref(), Op::SetThreadName { name })
|
||||
.await
|
||||
@@ -3218,6 +3246,7 @@ impl CodexMessageProcessor {
|
||||
return Err(invalid_request("gitInfo must include at least one field"));
|
||||
}
|
||||
|
||||
let _thread_list_state_permit = self.acquire_thread_list_state_permit().await?;
|
||||
let loaded_thread = self.thread_manager.get_thread(thread_uuid).await.ok();
|
||||
let mut state_db_ctx = loaded_thread.as_ref().and_then(|thread| thread.state_db());
|
||||
if state_db_ctx.is_none() {
|
||||
@@ -3414,6 +3443,13 @@ impl CodexMessageProcessor {
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadUnarchiveParams,
|
||||
) {
|
||||
let _thread_list_state_permit = match self.acquire_thread_list_state_permit().await {
|
||||
Ok(permit) => permit,
|
||||
Err(error) => {
|
||||
self.outgoing.send_error(request_id, error).await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
let result = self.thread_unarchive_response(params).await;
|
||||
let notification =
|
||||
result
|
||||
@@ -4143,6 +4179,13 @@ impl CodexMessageProcessor {
|
||||
return;
|
||||
}
|
||||
|
||||
let _thread_list_state_permit = match self.acquire_thread_list_state_permit().await {
|
||||
Ok(permit) => permit,
|
||||
Err(error) => {
|
||||
self.outgoing.send_error(request_id, error).await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
match self.resume_running_thread(&request_id, ¶ms).await {
|
||||
Ok(true) => return,
|
||||
Ok(false) => {}
|
||||
@@ -7237,6 +7280,7 @@ impl CodexMessageProcessor {
|
||||
pending_thread_unloads: Arc::clone(&self.pending_thread_unloads),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
thread_list_state_permit: self.thread_list_state_permit.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.to_path_buf(),
|
||||
},
|
||||
@@ -7354,6 +7398,7 @@ impl CodexMessageProcessor {
|
||||
pending_thread_unloads: Arc::clone(&self.pending_thread_unloads),
|
||||
analytics_events_client: self.analytics_events_client.clone(),
|
||||
thread_watch_manager: self.thread_watch_manager.clone(),
|
||||
thread_list_state_permit: self.thread_list_state_permit.clone(),
|
||||
fallback_model_provider: self.config.model_provider_id.clone(),
|
||||
codex_home: self.config.codex_home.to_path_buf(),
|
||||
},
|
||||
@@ -7402,6 +7447,7 @@ impl CodexMessageProcessor {
|
||||
pending_thread_unloads,
|
||||
analytics_events_client: _,
|
||||
thread_watch_manager,
|
||||
thread_list_state_permit,
|
||||
fallback_model_provider,
|
||||
codex_home,
|
||||
} = listener_task_context;
|
||||
@@ -7480,6 +7526,7 @@ impl CodexMessageProcessor {
|
||||
thread_outgoing,
|
||||
thread_state.clone(),
|
||||
thread_watch_manager.clone(),
|
||||
thread_list_state_permit.clone(),
|
||||
api_version,
|
||||
fallback_model_provider.clone(),
|
||||
codex_home.as_path(),
|
||||
|
||||
Reference in New Issue
Block a user