github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-01 16:17:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

[codex-analytics] emit terminal review events (#18748)

Browse Source 
## Why

Review telemetry should describe reviews as first-class events, not only
as counters denormalized onto terminal tool-item events. That lets us
analyze guardian and user reviews consistently across command execution,
file changes, permissions, and network access, while still preserving
the terminal item summaries that existing tool analytics need.

To make those review events accurate, analytics also needs the observed
completion time for each review and enough command metadata to
distinguish `shell` from `unified_exec` reviews.

## What changed

- emit generic `codex_review_event` rows for completed user and guardian
reviews, with review subjects, reviewer, trigger, terminal status,
resolution, and observed duration
- reduce approval request / response / abort facts into review events
for command execution, file change, and permissions flows
- keep denormalized review counts, final approval outcome, and
permission-request flags on terminal tool-item events for
item-associated reviews
- plumb review completion timing so user-review responses and aborts use
app-server-observed completion times, while guardian analytics reuse the
same terminal timestamps emitted on guardian assessment events
- carry command approval `source` through the protocol and app-server
layers so review analytics can distinguish `shell` from `unified_exec`
- add analytics coverage for user-review emission, guardian-review
emission, permission reviews that should not denormalize onto tool
items, item-summary isolation across threads, and the serialized
review-event shape

## Verification

- `cargo test -p codex-analytics`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18748).
* __->__ #18748
* #21434
* #18747
* #17090
* #17089
* #20514
This commit is contained in:
rhan-oai
2026-05-11 22:13:32 +00:00
committed by GitHub
parent aa9e8f0262
commit a175ddacc0
8 changed files with 1382 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/bespoke_event_handling.rs
+5 -1
View File
@@ -772,6 +772,7 @@ pub(crate) async fn apply_bespoke_event_handling(
requested_permissions,
request_cwd,
pending_request_id,
outgoing,
receiver: rx,
request_permissions_guard: permission_guard,
};
@@ -1745,11 +1746,12 @@ async fn on_request_permissions_response(
requested_permissions,
request_cwd,
pending_request_id,
outgoing,
receiver,
request_permissions_guard,
} = pending_response;
let response = receiver.await;
resolve_server_request_on_thread_listener(&thread_state, pending_request_id).await;
resolve_server_request_on_thread_listener(&thread_state, pending_request_id.clone()).await;
drop(request_permissions_guard);
let Some(response) = request_permissions_response_from_client_result(
requested_permissions,
@@ -1758,6 +1760,7 @@ async fn on_request_permissions_response(
) else {
return;
};
outgoing.track_effective_permissions_approval_response(pending_request_id, response.clone());
if let Err(err) = conversation
.submit(Op::RequestPermissionsResponse {
@@ -1775,6 +1778,7 @@ struct PendingRequestPermissionsResponse {
requested_permissions: CoreRequestPermissionProfile,
request_cwd: AbsolutePathBuf,
pending_request_id: RequestId,
outgoing: ThreadScopedOutgoingMessageSender,
receiver: oneshot::Receiver<ClientRequestResult>,
request_permissions_guard: ThreadWatchActiveGuard,
}
codex-rs/app-server/src/outgoing_message.rs
+45 -14
View File
@@ -13,9 +13,11 @@ use codex_app_server_protocol::Result;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
use codex_app_server_protocol::ServerRequestPayload;
use codex_app_server_protocol::ServerResponse;
use codex_otel::span_w3c_trace_context;
use codex_protocol::ThreadId;
use codex_protocol::protocol::W3cTraceContext;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use tokio::sync::Mutex;
use tokio::sync::mpsc;
use tokio::sync::oneshot;
@@ -141,6 +143,20 @@ impl ThreadScopedOutgoingMessageSender {
.await
}
pub(crate) fn track_effective_permissions_approval_response(
&self,
request_id: RequestId,
response: RequestPermissionsResponse,
) {
self.outgoing
.analytics_events_client
.track_effective_permissions_approval_response(
now_unix_timestamp_ms(),
request_id,
response,
);
}
pub(crate) async fn send_server_notification(&self, notification: ServerNotification) {
self.outgoing
.analytics_events_client
@@ -360,7 +376,9 @@ impl OutgoingMessageSender {
match entry {
Some((id, entry)) => {
let completed_at_ms = now_unix_timestamp_ms();
if let Ok(response) = entry.request.response_from_result(result.clone()) {
if let Ok(response) = entry.request.response_from_result(result.clone())
&& !matches!(response, ServerResponse::PermissionsRequestApproval { .. })
{
self.analytics_events_client
.track_server_response(completed_at_ms, response);
}
@@ -380,6 +398,8 @@ impl OutgoingMessageSender {
match entry {
Some((id, entry)) => {
warn!("client responded with error for {id:?}: {error:?}");
self.analytics_events_client
.track_server_request_aborted(now_unix_timestamp_ms(), id.clone());
if let Err(err) = entry.callback.send(Err(error)) {
warn!("could not notify callback for {id:?} due to: {err:?}");
}
@@ -391,7 +411,14 @@ impl OutgoingMessageSender {
}
pub(crate) async fn cancel_request(&self, id: &RequestId) -> bool {
self.take_request_callback(id).await.is_some()
let entry = self.take_request_callback(id).await;
if let Some((request_id, _entry)) = entry {
self.analytics_events_client
.track_server_request_aborted(now_unix_timestamp_ms(), request_id);
true
} else {
false
}
}
pub(crate) async fn cancel_all_requests(&self, error: Option<JSONRPCErrorError>) {
@@ -403,12 +430,14 @@ impl OutgoingMessageSender {
.collect::<Vec<_>>()
};
if let Some(error) = error {
for entry in entries {
if let Err(err) = entry.callback.send(Err(error.clone())) {
let request_id = entry.request.id();
warn!("could not notify callback for {request_id:?} due to: {err:?}");
}
for entry in entries {
self.analytics_events_client
.track_server_request_aborted(now_unix_timestamp_ms(), entry.request.id().clone());
if let Some(error) = error.as_ref()
&& let Err(err) = entry.callback.send(Err(error.clone()))
{
let request_id = entry.request.id();
warn!("could not notify callback for {request_id:?} due to: {err:?}");
}
}
}
@@ -459,12 +488,14 @@ impl OutgoingMessageSender {
entries
};
if let Some(error) = error {
for entry in entries {
if let Err(err) = entry.callback.send(Err(error.clone())) {
let request_id = entry.request.id();
warn!("could not notify callback for {request_id:?} due to: {err:?}",);
}
for entry in entries {
self.analytics_events_client
.track_server_request_aborted(now_unix_timestamp_ms(), entry.request.id().clone());
if let Some(error) = error.as_ref()
&& let Err(err) = entry.callback.send(Err(error.clone()))
{
let request_id = entry.request.id();
warn!("could not notify callback for {request_id:?} due to: {err:?}",);
}
}
}