mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Factor in-process request response alias
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -52,11 +52,13 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
|
||||
- Commit `d017d0fc3` fixed those two Windows-specific flakes, but the first in-process fuzzy-search harness revision introduced a new ordering regression on Linux.
|
||||
- Linux failure: `all::suite::fuzzy_file_search::test_fuzzy_file_search_session_update_works_without_waiting_for_start_response` received an error instead of a response because `sessionUpdate` could be enqueued ahead of `sessionStart`.
|
||||
- Commit `4195e7e80` fixed that ordering race, but the new public in-process request helpers returned a private type alias and tripped the `Lint/Build` matrix before the remaining test jobs could finish.
|
||||
- Commit `ef5a05fa3` narrowed those signatures back to public concrete types, but the same methods still expanded to a complex nested `Result` in every `cargo clippy` target. The follow-up replaces that inline type with a documented public alias so the request-order fix does not keep failing non-release `Lint/Build`.
|
||||
- Current patch set:
|
||||
- Pin the standalone shell test to `cmd.exe` on Windows so it validates reference-context isolation without depending on PowerShell startup behavior.
|
||||
- Replace the fuzzy-file-search suite's spawned `codex-app-server` harness with the in-process app-server runtime so the tests still exercise request/notification behavior without the flaky stdio startup path.
|
||||
- Preserve in-process request order by enqueueing requests synchronously and storing a pending-response handle instead of spawning `sender.request(...)` tasks that can race.
|
||||
- Inline the public in-process request return types so the request-order fix no longer leaks a private alias through the `app-server` public API.
|
||||
- Expose that request-response shape through a documented public alias so `cargo clippy -D warnings` does not reject the in-process helper API for `type_complexity`.
|
||||
- Rationale: these failures are test-harness flakes, not product behaviors. The fixes keep the assertions intact and remove environment-sensitive startup and ordering hazards instead of stretching timeouts.
|
||||
|
||||
## Constraints
|
||||
@@ -88,3 +90,4 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
|
||||
| `495ef3f76` | Expose nextest failure tail in CI | failed | Run `23083992418` failed in both Windows `Tests` jobs. The new annotations identified `codex::tests::run_user_shell_command_does_not_set_reference_context_item` timing out on Windows x64 and `all::suite::fuzzy_file_search::test_fuzzy_file_search_session_multiple_query_updates_work` timing out during app-server `initialize` on Windows arm64. |
|
||||
| `d017d0fc3` | Stabilize Windows shell and fuzzy search tests | failed | Run `23084639316` fixed the original Windows-targeted failures, but introduced a new failure in `Tests — ubuntu-24.04 - x86_64-unknown-linux-gnu`: `all::suite::fuzzy_file_search::test_fuzzy_file_search_session_update_works_without_waiting_for_start_response`. The in-process harness used spawned `sender.request(...)` tasks, which made request submission order nondeterministic and let `sessionUpdate` race ahead of `sessionStart`. `Bazel (experimental)` also failed on macOS in the same patch window, and the still-running Windows jobs were superseded by the next follow-up commit. |
|
||||
| `4195e7e80` | Preserve in-process fuzzy search request ordering | failed | Run `23084952538` proved the Linux ordering fix was necessary, but the follow-up widened the `app-server` public API with methods that returned a private type alias. `Lint/Build` failed on Linux and macOS before the remaining `Tests` jobs finished, so the next follow-up narrows the public signatures back to concrete types and reuses the same request-order behavior. |
|
||||
| `ef5a05fa3` | Inline in-process request response types | failed | Run `23085144063` cleared the private-alias compile break, but every non-release `Lint/Build` target still failed in `cargo clippy` while release builds passed. The next follow-up replaces the inline `IoResult<Result<...>>` signatures with a documented public alias, which is the smallest code change consistent with the cross-target `cargo clippy` pattern. |
|
||||
|
||||
@@ -92,15 +92,16 @@ const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
/// Default bounded channel capacity for in-process runtime queues.
|
||||
pub const DEFAULT_IN_PROCESS_CHANNEL_CAPACITY: usize = CHANNEL_CAPACITY;
|
||||
|
||||
type PendingClientRequestResponse = std::result::Result<Result, JSONRPCErrorError>;
|
||||
/// JSON-RPC application response returned by in-process client requests.
|
||||
pub type InProcessRequestResponse = std::result::Result<Result, JSONRPCErrorError>;
|
||||
|
||||
/// Handle for a request that has been enqueued but whose response has not yet been awaited.
|
||||
pub struct PendingInProcessRequest {
|
||||
response_rx: oneshot::Receiver<PendingClientRequestResponse>,
|
||||
response_rx: oneshot::Receiver<InProcessRequestResponse>,
|
||||
}
|
||||
|
||||
impl PendingInProcessRequest {
|
||||
pub async fn recv(self) -> IoResult<std::result::Result<Result, JSONRPCErrorError>> {
|
||||
pub async fn recv(self) -> IoResult<InProcessRequestResponse> {
|
||||
self.response_rx.await.map_err(|err| {
|
||||
IoError::new(
|
||||
ErrorKind::BrokenPipe,
|
||||
@@ -187,7 +188,7 @@ pub enum InProcessServerEvent {
|
||||
enum InProcessClientMessage {
|
||||
Request {
|
||||
request: Box<ClientRequest>,
|
||||
response_tx: oneshot::Sender<PendingClientRequestResponse>,
|
||||
response_tx: oneshot::Sender<InProcessRequestResponse>,
|
||||
},
|
||||
Notification {
|
||||
notification: ClientNotification,
|
||||
@@ -225,10 +226,7 @@ impl InProcessClientSender {
|
||||
Ok(PendingInProcessRequest { response_rx })
|
||||
}
|
||||
|
||||
pub async fn request(
|
||||
&self,
|
||||
request: ClientRequest,
|
||||
) -> IoResult<std::result::Result<Result, JSONRPCErrorError>> {
|
||||
pub async fn request(&self, request: ClientRequest) -> IoResult<InProcessRequestResponse> {
|
||||
self.start_request(request)?.recv().await
|
||||
}
|
||||
|
||||
@@ -293,10 +291,7 @@ impl InProcessClientHandle {
|
||||
/// request IDs unique among concurrent requests; reusing an in-flight ID
|
||||
/// produces an `INVALID_REQUEST` response and can make request routing
|
||||
/// ambiguous in the caller.
|
||||
pub async fn request(
|
||||
&self,
|
||||
request: ClientRequest,
|
||||
) -> IoResult<std::result::Result<Result, JSONRPCErrorError>> {
|
||||
pub async fn request(&self, request: ClientRequest) -> IoResult<InProcessRequestResponse> {
|
||||
self.client.request(request).await
|
||||
}
|
||||
|
||||
@@ -515,7 +510,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle {
|
||||
processor.connection_closed(IN_PROCESS_CONNECTION_ID).await;
|
||||
});
|
||||
let mut pending_request_responses =
|
||||
HashMap::<RequestId, oneshot::Sender<PendingClientRequestResponse>>::new();
|
||||
HashMap::<RequestId, oneshot::Sender<InProcessRequestResponse>>::new();
|
||||
let mut shutdown_ack = None;
|
||||
|
||||
loop {
|
||||
|
||||
Reference in New Issue
Block a user