mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex-analytics] plumb protocol-native review timing (#21434)
## Why We want terminal tool review analytics, but the reducer should not stamp review timing from its own wall clock. This PR plumbs review timing through the real protocol and app-server seams so downstream analytics can consume the emitter's timestamps directly. Guardian reviews keep their enriched `started_at` / `completed_at` analytics fields by deriving those legacy second-based values from the same protocol-native millisecond lifecycle timestamps, rather than sampling a separate analytics clock. ## What changed - add `started_at_ms` to user approval request payloads - add `started_at_ms` / `completed_at_ms` to guardian review notifications - preserve Guardian review `started_at` / `completed_at` enrichment from the protocol-native timing source - stamp typed `ServerResponse` analytics facts with app-server-observed `completed_at_ms` - thread the new timing fields through core, protocol, app-server, TUI, and analytics fixtures ## Verification - `cargo test -p codex-app-server outgoing_message --manifest-path codex-rs/Cargo.toml` - `cargo test -p codex-app-server-protocol guardian --manifest-path codex-rs/Cargo.toml` - `cargo test -p codex-tui guardian --manifest-path codex-rs/Cargo.toml` - `cargo test -p codex-analytics analytics_client_tests --manifest-path codex-rs/Cargo.toml` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/21434). * #18748 * __->__ #21434 * #18747 * #17090 * #17089 * #20514
This commit is contained in:
committed by
GitHub
Unverified
parent
af16baa549
commit
99016ec732
@@ -340,8 +340,9 @@ impl AnalyticsEventsClient {
|
||||
});
|
||||
}
|
||||
|
||||
pub fn track_server_response(&self, response: ServerResponse) {
|
||||
pub fn track_server_response(&self, completed_at_ms: u64, response: ServerResponse) {
|
||||
self.record_fact(AnalyticsFact::ServerResponse {
|
||||
completed_at_ms,
|
||||
response: Box::new(response),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ use crate::facts::TurnStatus;
|
||||
use crate::facts::TurnSteerRejectionReason;
|
||||
use crate::facts::TurnSteerResult;
|
||||
use crate::facts::TurnSubmissionType;
|
||||
use crate::now_unix_millis;
|
||||
use crate::now_unix_seconds;
|
||||
use codex_app_server_protocol::CodexErrorInfo;
|
||||
use codex_app_server_protocol::CommandExecutionSource;
|
||||
@@ -261,7 +262,7 @@ pub struct GuardianReviewTrackContext {
|
||||
approval_request_source: GuardianApprovalRequestSource,
|
||||
reviewed_action: GuardianReviewedAction,
|
||||
review_timeout_ms: u64,
|
||||
started_at: u64,
|
||||
pub started_at_ms: u64,
|
||||
started_instant: Instant,
|
||||
}
|
||||
|
||||
@@ -283,7 +284,7 @@ impl GuardianReviewTrackContext {
|
||||
approval_request_source,
|
||||
reviewed_action,
|
||||
review_timeout_ms,
|
||||
started_at: now_unix_seconds(),
|
||||
started_at_ms: now_unix_millis(),
|
||||
started_instant: Instant::now(),
|
||||
}
|
||||
}
|
||||
@@ -316,7 +317,7 @@ impl GuardianReviewTrackContext {
|
||||
tool_call_count: None,
|
||||
time_to_first_token_ms: result.time_to_first_token_ms,
|
||||
completion_latency_ms: Some(self.started_instant.elapsed().as_millis() as u64),
|
||||
started_at: self.started_at,
|
||||
started_at: self.started_at_ms / 1_000,
|
||||
completed_at: Some(now_unix_seconds()),
|
||||
input_tokens: result.token_usage.as_ref().map(|usage| usage.input_tokens),
|
||||
cached_input_tokens: result
|
||||
|
||||
@@ -296,6 +296,7 @@ pub(crate) enum AnalyticsFact {
|
||||
request: Box<ServerRequest>,
|
||||
},
|
||||
ServerResponse {
|
||||
completed_at_ms: u64,
|
||||
response: Box<ServerResponse>,
|
||||
},
|
||||
Notification(Box<ServerNotification>),
|
||||
|
||||
@@ -325,6 +325,7 @@ impl AnalyticsReducer {
|
||||
} => {}
|
||||
AnalyticsFact::ServerResponse {
|
||||
response: _response,
|
||||
..
|
||||
} => {}
|
||||
AnalyticsFact::Custom(input) => match input {
|
||||
CustomAnalyticsFact::SubAgentThreadStarted(input) => {
|
||||
|
||||
+6
@@ -593,6 +593,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -602,6 +607,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
+6
@@ -18,6 +18,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -27,6 +32,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
+6
@@ -297,6 +297,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -308,6 +313,7 @@
|
||||
"cwd",
|
||||
"itemId",
|
||||
"permissions",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
@@ -1963,6 +1963,11 @@
|
||||
"action": {
|
||||
"$ref": "#/definitions/GuardianApprovalReviewAction"
|
||||
},
|
||||
"completedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review completed.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"decisionSource": {
|
||||
"$ref": "#/definitions/AutoReviewDecisionSource"
|
||||
},
|
||||
@@ -1973,6 +1978,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -1989,9 +1999,11 @@
|
||||
},
|
||||
"required": [
|
||||
"action",
|
||||
"completedAtMs",
|
||||
"decisionSource",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -2010,6 +2022,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -2028,6 +2045,7 @@
|
||||
"action",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
@@ -417,6 +417,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -426,6 +431,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -598,6 +604,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -607,6 +618,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -1587,6 +1599,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -1598,6 +1615,7 @@
|
||||
"cwd",
|
||||
"itemId",
|
||||
"permissions",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
+36
@@ -2146,6 +2146,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -2155,6 +2160,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -2411,6 +2417,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -2420,6 +2431,7 @@
|
||||
},
|
||||
"required": [
|
||||
"itemId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -3591,6 +3603,11 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this approval request started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"threadId": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -3602,6 +3619,7 @@
|
||||
"cwd",
|
||||
"itemId",
|
||||
"permissions",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -9878,6 +9896,11 @@
|
||||
"action": {
|
||||
"$ref": "#/definitions/v2/GuardianApprovalReviewAction"
|
||||
},
|
||||
"completedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review completed.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"decisionSource": {
|
||||
"$ref": "#/definitions/v2/AutoReviewDecisionSource"
|
||||
},
|
||||
@@ -9888,6 +9911,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -9904,9 +9932,11 @@
|
||||
},
|
||||
"required": [
|
||||
"action",
|
||||
"completedAtMs",
|
||||
"decisionSource",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -9927,6 +9957,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -9945,6 +9980,7 @@
|
||||
"action",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
+18
@@ -6489,6 +6489,11 @@
|
||||
"action": {
|
||||
"$ref": "#/definitions/GuardianApprovalReviewAction"
|
||||
},
|
||||
"completedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review completed.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"decisionSource": {
|
||||
"$ref": "#/definitions/AutoReviewDecisionSource"
|
||||
},
|
||||
@@ -6499,6 +6504,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -6515,9 +6525,11 @@
|
||||
},
|
||||
"required": [
|
||||
"action",
|
||||
"completedAtMs",
|
||||
"decisionSource",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
@@ -6538,6 +6550,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -6556,6 +6573,7 @@
|
||||
"action",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
Generated
+12
@@ -574,6 +574,11 @@
|
||||
"action": {
|
||||
"$ref": "#/definitions/GuardianApprovalReviewAction"
|
||||
},
|
||||
"completedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review completed.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"decisionSource": {
|
||||
"$ref": "#/definitions/AutoReviewDecisionSource"
|
||||
},
|
||||
@@ -584,6 +589,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -600,9 +610,11 @@
|
||||
},
|
||||
"required": [
|
||||
"action",
|
||||
"completedAtMs",
|
||||
"decisionSource",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
Generated
+6
@@ -574,6 +574,11 @@
|
||||
"description": "Stable identifier for this review.",
|
||||
"type": "string"
|
||||
},
|
||||
"startedAtMs": {
|
||||
"description": "Unix timestamp (in milliseconds) when this review started.",
|
||||
"format": "int64",
|
||||
"type": "integer"
|
||||
},
|
||||
"targetItemId": {
|
||||
"description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.",
|
||||
"type": [
|
||||
@@ -592,6 +597,7 @@
|
||||
"action",
|
||||
"review",
|
||||
"reviewId",
|
||||
"startedAtMs",
|
||||
"threadId",
|
||||
"turnId"
|
||||
],
|
||||
|
||||
Generated
+3
@@ -8,6 +8,9 @@ import type { NetworkApprovalContext } from "./NetworkApprovalContext";
|
||||
import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
|
||||
|
||||
export type CommandExecutionRequestApprovalParams = {threadId: string, turnId: string, itemId: string, /**
|
||||
* Unix timestamp (in milliseconds) when this approval request started.
|
||||
*/
|
||||
startedAtMs: number, /**
|
||||
* Unique identifier for this specific approval callback.
|
||||
*
|
||||
* For regular shell/unified_exec approvals, this is null.
|
||||
|
||||
+4
@@ -3,6 +3,10 @@
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
|
||||
export type FileChangeRequestApprovalParams = { threadId: string, turnId: string, itemId: string,
|
||||
/**
|
||||
* Unix timestamp (in milliseconds) when this approval request started.
|
||||
*/
|
||||
startedAtMs: number,
|
||||
/**
|
||||
* Optional explanatory reason (e.g. request for extra write access).
|
||||
*/
|
||||
|
||||
codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts
Generated
+8
@@ -10,6 +10,14 @@ import type { GuardianApprovalReviewAction } from "./GuardianApprovalReviewActio
|
||||
* shape is expected to change soon.
|
||||
*/
|
||||
export type ItemGuardianApprovalReviewCompletedNotification = { threadId: string, turnId: string,
|
||||
/**
|
||||
* Unix timestamp (in milliseconds) when this review started.
|
||||
*/
|
||||
startedAtMs: number,
|
||||
/**
|
||||
* Unix timestamp (in milliseconds) when this review completed.
|
||||
*/
|
||||
completedAtMs: number,
|
||||
/**
|
||||
* Stable identifier for this review.
|
||||
*/
|
||||
|
||||
Generated
+4
@@ -9,6 +9,10 @@ import type { GuardianApprovalReviewAction } from "./GuardianApprovalReviewActio
|
||||
* shape is expected to change soon.
|
||||
*/
|
||||
export type ItemGuardianApprovalReviewStartedNotification = { threadId: string, turnId: string,
|
||||
/**
|
||||
* Unix timestamp (in milliseconds) when this review started.
|
||||
*/
|
||||
startedAtMs: number,
|
||||
/**
|
||||
* Stable identifier for this review.
|
||||
*/
|
||||
|
||||
+5
-1
@@ -4,4 +4,8 @@
|
||||
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
|
||||
import type { RequestPermissionProfile } from "./RequestPermissionProfile";
|
||||
|
||||
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, cwd: AbsolutePathBuf, reason: string | null, permissions: RequestPermissionProfile, };
|
||||
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string,
|
||||
/**
|
||||
* Unix timestamp (in milliseconds) when this approval request started.
|
||||
*/
|
||||
startedAtMs: number, cwd: AbsolutePathBuf, reason: string | null, permissions: RequestPermissionProfile, };
|
||||
|
||||
@@ -2967,6 +2967,7 @@ mod tests {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -243,6 +243,7 @@ pub fn guardian_auto_approval_review_notification(
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id,
|
||||
review_id: assessment.id.clone(),
|
||||
started_at_ms: assessment.started_at_ms,
|
||||
target_item_id: assessment.target_item_id.clone(),
|
||||
review,
|
||||
action,
|
||||
@@ -258,6 +259,10 @@ pub fn guardian_auto_approval_review_notification(
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id,
|
||||
review_id: assessment.id.clone(),
|
||||
started_at_ms: assessment.started_at_ms,
|
||||
completed_at_ms: assessment
|
||||
.completed_at_ms
|
||||
.unwrap_or(assessment.started_at_ms),
|
||||
target_item_id: assessment.target_item_id.clone(),
|
||||
decision_source: assessment
|
||||
.decision_source
|
||||
|
||||
@@ -2143,6 +2143,8 @@ mod tests {
|
||||
id: "review-guardian-exec".into(),
|
||||
target_item_id: Some("guardian-exec".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -2160,6 +2162,8 @@ mod tests {
|
||||
id: "review-guardian-exec".into(),
|
||||
target_item_id: Some("guardian-exec".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: Some(1_042),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_level: Some(codex_protocol::protocol::GuardianRiskLevel::High),
|
||||
user_authorization: Some(codex_protocol::protocol::GuardianUserAuthorization::Low),
|
||||
@@ -2222,6 +2226,8 @@ mod tests {
|
||||
id: "review-guardian-execve".into(),
|
||||
target_item_id: Some("guardian-execve".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 2_000,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -2525,6 +2531,7 @@ mod tests {
|
||||
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id: "patch-call".into(),
|
||||
turn_id: turn_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
changes: [(
|
||||
PathBuf::from("README.md"),
|
||||
codex_protocol::protocol::FileChange::Add {
|
||||
|
||||
@@ -1073,6 +1073,9 @@ pub struct ItemStartedNotification {
|
||||
pub struct ItemGuardianApprovalReviewStartedNotification {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
/// Unix timestamp (in milliseconds) when this review started.
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
/// Stable identifier for this review.
|
||||
pub review_id: String,
|
||||
/// Identifier for the reviewed item or tool call when one exists.
|
||||
@@ -1099,6 +1102,12 @@ pub struct ItemGuardianApprovalReviewStartedNotification {
|
||||
pub struct ItemGuardianApprovalReviewCompletedNotification {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
/// Unix timestamp (in milliseconds) when this review started.
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
/// Unix timestamp (in milliseconds) when this review completed.
|
||||
#[ts(type = "number")]
|
||||
pub completed_at_ms: i64,
|
||||
/// Stable identifier for this review.
|
||||
pub review_id: String,
|
||||
/// Identifier for the reviewed item or tool call when one exists.
|
||||
@@ -1248,6 +1257,9 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
/// Unix timestamp (in milliseconds) when this approval request started.
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
/// Unique identifier for this specific approval callback.
|
||||
///
|
||||
/// For regular shell/unified_exec approvals, this is null.
|
||||
@@ -1321,6 +1333,9 @@ pub struct FileChangeRequestApprovalParams {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
/// Unix timestamp (in milliseconds) when this approval request started.
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
/// Optional explanatory reason (e.g. request for extra write access).
|
||||
#[ts(optional = nullable)]
|
||||
pub reason: Option<String>,
|
||||
|
||||
@@ -826,6 +826,9 @@ pub struct PermissionsRequestApprovalParams {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
/// Unix timestamp (in milliseconds) when this approval request started.
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub reason: Option<String>,
|
||||
pub permissions: RequestPermissionProfile,
|
||||
|
||||
@@ -277,6 +277,7 @@ fn command_execution_request_approval_rejects_relative_additional_permission_pat
|
||||
"threadId": "thr_123",
|
||||
"turnId": "turn_123",
|
||||
"itemId": "call_123",
|
||||
"startedAtMs": 1,
|
||||
"command": "cat file",
|
||||
"cwd": absolute_path_string("tmp"),
|
||||
"commandActions": null,
|
||||
@@ -317,6 +318,7 @@ fn permissions_request_approval_uses_request_permission_profile() {
|
||||
"threadId": "thr_123",
|
||||
"turnId": "turn_123",
|
||||
"itemId": "call_123",
|
||||
"startedAtMs": 1,
|
||||
"cwd": absolute_path_string("repo"),
|
||||
"reason": "Select a workspace root",
|
||||
"permissions": {
|
||||
@@ -379,6 +381,7 @@ fn permissions_request_approval_rejects_macos_permissions() {
|
||||
"threadId": "thr_123",
|
||||
"turnId": "turn_123",
|
||||
"itemId": "call_123",
|
||||
"startedAtMs": 1,
|
||||
"cwd": absolute_path_string("repo"),
|
||||
"reason": "Select a workspace root",
|
||||
"permissions": {
|
||||
|
||||
@@ -1945,6 +1945,7 @@ impl CodexClient {
|
||||
thread_id,
|
||||
turn_id,
|
||||
item_id,
|
||||
started_at_ms: _,
|
||||
approval_id,
|
||||
reason,
|
||||
network_approval_context,
|
||||
@@ -2020,6 +2021,7 @@ impl CodexClient {
|
||||
thread_id,
|
||||
turn_id,
|
||||
item_id,
|
||||
started_at_ms: _,
|
||||
reason,
|
||||
grant_root,
|
||||
} = params;
|
||||
|
||||
@@ -511,6 +511,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event.turn_id.clone(),
|
||||
item_id: item_id.clone(),
|
||||
started_at_ms: event.started_at_ms,
|
||||
reason: event.reason.clone(),
|
||||
grant_root: event.grant_root.clone(),
|
||||
};
|
||||
@@ -542,6 +543,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
call_id,
|
||||
approval_id,
|
||||
turn_id,
|
||||
started_at_ms,
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
@@ -615,6 +617,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: turn_id.clone(),
|
||||
item_id: call_id.clone(),
|
||||
started_at_ms,
|
||||
approval_id: approval_id.clone(),
|
||||
reason,
|
||||
network_approval_context,
|
||||
@@ -764,6 +767,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: request.turn_id.clone(),
|
||||
item_id: request.call_id.clone(),
|
||||
started_at_ms: request.started_at_ms,
|
||||
cwd: request_cwd.clone(),
|
||||
reason: request.reason,
|
||||
permissions: request.permissions.into(),
|
||||
@@ -2249,6 +2253,9 @@ mod tests {
|
||||
id: format!("review-{id}"),
|
||||
target_item_id: Some(id.to_string()),
|
||||
turn_id: turn_id.to_string(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: (!matches!(status, GuardianAssessmentStatus::InProgress))
|
||||
.then_some(1_042),
|
||||
status,
|
||||
risk_level,
|
||||
user_authorization,
|
||||
@@ -2313,6 +2320,8 @@ mod tests {
|
||||
id: "review-1".to_string(),
|
||||
target_item_id: Some("item-1".to_string()),
|
||||
turn_id: String::new(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: None,
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -2326,6 +2335,7 @@ mod tests {
|
||||
ServerNotification::ItemGuardianApprovalReviewStarted(payload) => {
|
||||
assert_eq!(payload.thread_id, conversation_id.to_string());
|
||||
assert_eq!(payload.turn_id, "turn-from-event");
|
||||
assert_eq!(payload.started_at_ms, 1_000);
|
||||
assert_eq!(payload.review_id, "review-1");
|
||||
assert_eq!(payload.target_item_id.as_deref(), Some("item-1"));
|
||||
assert_eq!(
|
||||
@@ -2356,6 +2366,8 @@ mod tests {
|
||||
id: "review-2".to_string(),
|
||||
target_item_id: Some("item-2".to_string()),
|
||||
turn_id: "turn-from-assessment".to_string(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: Some(1_042),
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::Denied,
|
||||
risk_level: Some(codex_protocol::protocol::GuardianRiskLevel::High),
|
||||
user_authorization: Some(codex_protocol::protocol::GuardianUserAuthorization::Low),
|
||||
@@ -2371,6 +2383,8 @@ mod tests {
|
||||
ServerNotification::ItemGuardianApprovalReviewCompleted(payload) => {
|
||||
assert_eq!(payload.thread_id, conversation_id.to_string());
|
||||
assert_eq!(payload.turn_id, "turn-from-assessment");
|
||||
assert_eq!(payload.started_at_ms, 1_000);
|
||||
assert_eq!(payload.completed_at_ms, 1_042);
|
||||
assert_eq!(payload.review_id, "review-2");
|
||||
assert_eq!(payload.target_item_id.as_deref(), Some("item-2"));
|
||||
assert_eq!(payload.decision_source, AutoReviewDecisionSource::Agent);
|
||||
@@ -2406,6 +2420,8 @@ mod tests {
|
||||
id: "review-3".to_string(),
|
||||
target_item_id: None,
|
||||
turn_id: "turn-from-assessment".to_string(),
|
||||
started_at_ms: 1_000,
|
||||
completed_at_ms: Some(1_042),
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::Aborted,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
|
||||
@@ -2,6 +2,8 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicI64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_app_server_protocol::ClientResponsePayload;
|
||||
@@ -357,8 +359,10 @@ 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()) {
|
||||
self.analytics_events_client.track_server_response(response);
|
||||
self.analytics_events_client
|
||||
.track_server_response(completed_at_ms, response);
|
||||
}
|
||||
if let Err(err) = entry.callback.send(Ok(result)) {
|
||||
warn!("could not notify callback for {id:?} due to: {err:?}");
|
||||
@@ -648,6 +652,15 @@ impl OutgoingMessageSender {
|
||||
}
|
||||
}
|
||||
|
||||
fn now_unix_timestamp_ms() -> u64 {
|
||||
SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_millis()
|
||||
.try_into()
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::time::Duration;
|
||||
@@ -903,6 +916,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
@@ -1195,6 +1209,7 @@ mod tests {
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "call-2".to_string(),
|
||||
started_at_ms: 0,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
},
|
||||
|
||||
@@ -258,6 +258,7 @@ async fn command_execution_request_approval_strips_additional_permissions_withou
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: None,
|
||||
reason: Some("Need extra read access".to_string()),
|
||||
network_approval_context: None,
|
||||
@@ -322,6 +323,7 @@ async fn command_execution_request_approval_keeps_additional_permissions_with_ca
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: None,
|
||||
reason: Some("Need extra read access".to_string()),
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -225,6 +225,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
|
||||
RequestPermissionsEvent {
|
||||
call_id: request_call_id,
|
||||
turn_id: "child-turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
reason: Some("need access".to_string()),
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
@@ -313,6 +314,7 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
|
||||
call_id: "command-item-1".to_string(),
|
||||
approval_id: Some("callback-approval-1".to_string()),
|
||||
turn_id: "child-turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
command: vec!["rm".to_string(), "-rf".to_string(), "tmp".to_string()],
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
reason: Some("unsafe subcommand".to_string()),
|
||||
|
||||
@@ -23,6 +23,7 @@ use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::turn_timing::now_unix_timestamp_ms;
|
||||
|
||||
use super::GUARDIAN_REVIEW_TIMEOUT;
|
||||
use super::GUARDIAN_REVIEWER_NAME;
|
||||
@@ -251,6 +252,7 @@ async fn run_guardian_review(
|
||||
guardian_reviewed_action(&request),
|
||||
GUARDIAN_REVIEW_TIMEOUT.as_millis() as u64,
|
||||
);
|
||||
let started_at_ms = review_tracking.started_at_ms.try_into().unwrap_or_default();
|
||||
session
|
||||
.send_event(
|
||||
turn.as_ref(),
|
||||
@@ -258,6 +260,8 @@ async fn run_guardian_review(
|
||||
id: review_id.clone(),
|
||||
target_item_id: target_item_id.clone(),
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
started_at_ms,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -289,6 +293,8 @@ async fn run_guardian_review(
|
||||
id: review_id,
|
||||
target_item_id,
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
started_at_ms,
|
||||
completed_at_ms: Some(now_unix_timestamp_ms()),
|
||||
status: GuardianAssessmentStatus::Aborted,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -372,6 +378,8 @@ async fn run_guardian_review(
|
||||
id: review_id,
|
||||
target_item_id,
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
started_at_ms,
|
||||
completed_at_ms: Some(now_unix_timestamp_ms()),
|
||||
status: GuardianAssessmentStatus::TimedOut,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -402,6 +410,8 @@ async fn run_guardian_review(
|
||||
id: review_id,
|
||||
target_item_id,
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
started_at_ms,
|
||||
completed_at_ms: Some(now_unix_timestamp_ms()),
|
||||
status: GuardianAssessmentStatus::Aborted,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -495,6 +505,8 @@ async fn run_guardian_review(
|
||||
id: review_id,
|
||||
target_item_id,
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
started_at_ms,
|
||||
completed_at_ms: Some(now_unix_timestamp_ms()),
|
||||
status,
|
||||
risk_level: Some(assessment.risk_level),
|
||||
user_authorization: Some(assessment.user_authorization),
|
||||
|
||||
@@ -1956,6 +1956,7 @@ impl Session {
|
||||
call_id,
|
||||
approval_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
started_at_ms: now_unix_timestamp_ms(),
|
||||
command,
|
||||
cwd,
|
||||
reason,
|
||||
@@ -2002,6 +2003,7 @@ impl Session {
|
||||
let event = EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
started_at_ms: now_unix_timestamp_ms(),
|
||||
changes,
|
||||
reason,
|
||||
grant_root,
|
||||
@@ -2165,6 +2167,7 @@ impl Session {
|
||||
let event = EventMsg::RequestPermissions(RequestPermissionsEvent {
|
||||
call_id: call_id.clone(),
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
started_at_ms: now_unix_timestamp_ms(),
|
||||
reason: args.reason,
|
||||
permissions: requested_permissions,
|
||||
cwd: Some(cwd),
|
||||
|
||||
@@ -222,6 +222,7 @@ async fn run_codex_tool_session_inner(
|
||||
let approval_id = ev.effective_approval_id();
|
||||
let ExecApprovalRequestEvent {
|
||||
turn_id: _,
|
||||
started_at_ms: _,
|
||||
command,
|
||||
cwd,
|
||||
call_id,
|
||||
@@ -278,6 +279,7 @@ async fn run_codex_tool_session_inner(
|
||||
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
|
||||
call_id,
|
||||
turn_id: _,
|
||||
started_at_ms: _,
|
||||
reason,
|
||||
grant_root,
|
||||
changes,
|
||||
|
||||
@@ -187,6 +187,12 @@ pub struct GuardianAssessmentEvent {
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
#[serde(default)]
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional, type = "number")]
|
||||
pub completed_at_ms: Option<i64>,
|
||||
pub status: GuardianAssessmentStatus,
|
||||
/// Coarse risk label. Omitted while the assessment is in progress.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -223,6 +229,8 @@ pub struct ExecApprovalRequestEvent {
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
/// The command to be executed.
|
||||
pub command: Vec<String>,
|
||||
/// The command's working directory.
|
||||
@@ -370,6 +378,8 @@ pub struct ApplyPatchApprovalRequestEvent {
|
||||
/// Uses `#[serde(default)]` for backwards compatibility with older senders.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
pub changes: HashMap<PathBuf, FileChange>,
|
||||
/// Optional explanatory reason (e.g. request for extra write access).
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -71,6 +71,8 @@ pub struct RequestPermissionsEvent {
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
#[ts(type = "number")]
|
||||
pub started_at_ms: i64,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reason: Option<String>,
|
||||
pub permissions: RequestPermissionProfile,
|
||||
|
||||
@@ -216,6 +216,68 @@ async fn load_rollout_items_skips_legacy_ghost_snapshot_lines() -> std::io::Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_rollout_items_preserves_legacy_guardian_assessment_lines() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let rollout_path = home.path().join("rollout.jsonl");
|
||||
let mut file = File::create(&rollout_path)?;
|
||||
let thread_id = ThreadId::new();
|
||||
let ts = "2025-01-03T12:00:00Z";
|
||||
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "session_meta",
|
||||
"payload": {
|
||||
"id": thread_id,
|
||||
"timestamp": ts,
|
||||
"cwd": ".",
|
||||
"originator": "test_originator",
|
||||
"cli_version": "test_version",
|
||||
"source": "cli",
|
||||
"model_provider": "test-provider",
|
||||
},
|
||||
})
|
||||
)?;
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "event_msg",
|
||||
"payload": {
|
||||
"type": "guardian_assessment",
|
||||
"id": "guardian-1",
|
||||
"turn_id": "turn-1",
|
||||
"status": "in_progress",
|
||||
"action": {
|
||||
"type": "command",
|
||||
"source": "shell",
|
||||
"command": "rm -rf /tmp/guardian",
|
||||
"cwd": if cfg!(windows) { r"C:\tmp" } else { "/tmp" },
|
||||
},
|
||||
},
|
||||
})
|
||||
)?;
|
||||
|
||||
let (items, loaded_thread_id, parse_errors) =
|
||||
RolloutRecorder::load_rollout_items(&rollout_path).await?;
|
||||
|
||||
assert_eq!(loaded_thread_id, Some(thread_id));
|
||||
assert_eq!(parse_errors, 0);
|
||||
assert_eq!(items.len(), 2);
|
||||
let RolloutItem::EventMsg(EventMsg::GuardianAssessment(assessment)) = &items[1] else {
|
||||
panic!("expected guardian assessment rollout item");
|
||||
};
|
||||
assert_eq!(assessment.id, "guardian-1");
|
||||
assert_eq!(assessment.turn_id, "turn-1");
|
||||
assert_eq!(assessment.started_at_ms, 0);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_rollout_items_filters_legacy_ghost_snapshots_from_compaction_history()
|
||||
-> std::io::Result<()> {
|
||||
|
||||
@@ -429,6 +429,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "call-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: Some("approval-1".to_string()),
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
@@ -481,6 +482,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "perm-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }),
|
||||
reason: None,
|
||||
permissions: serde_json::from_value(json!({
|
||||
@@ -686,6 +688,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "patch-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
},
|
||||
@@ -715,6 +718,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "call-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: Some("approval-1".to_string()),
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -612,6 +612,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: turn_id.to_string(),
|
||||
item_id: call_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: approval_id.map(str::to_string),
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
@@ -633,6 +634,7 @@ mod tests {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: turn_id.to_string(),
|
||||
item_id: call_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
reason: None,
|
||||
grant_root: None,
|
||||
},
|
||||
|
||||
@@ -2659,6 +2659,7 @@ async fn inactive_thread_file_change_approval_recovers_buffered_changes() {
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: "turn-approval".to_string(),
|
||||
item_id: "patch-approval".to_string(),
|
||||
started_at_ms: 0,
|
||||
reason: Some("command failed; retry without sandbox?".to_string()),
|
||||
grant_root: None,
|
||||
},
|
||||
@@ -2709,6 +2710,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: "turn-approval".to_string(),
|
||||
item_id: "call-approval".to_string(),
|
||||
started_at_ms: 0,
|
||||
cwd: test_absolute_path("/tmp"),
|
||||
reason: Some("Need access to .git".to_string()),
|
||||
permissions: codex_app_server_protocol::RequestPermissionProfile {
|
||||
@@ -4259,6 +4261,7 @@ fn exec_approval_request(
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: turn_id.to_string(),
|
||||
item_id: item_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: approval_id.map(str::to_string),
|
||||
reason: Some("needs approval".to_string()),
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -465,6 +465,7 @@ mod tests {
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: turn_id.to_string(),
|
||||
item_id: item_id.to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: approval_id.map(str::to_string),
|
||||
reason: Some("needs approval".to_string()),
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -88,6 +88,8 @@ mod tests {
|
||||
id: format!("review-{id}"),
|
||||
target_item_id: None,
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
|
||||
@@ -1637,6 +1637,7 @@ fn request_permissions_from_params(
|
||||
RequestPermissionsEvent {
|
||||
turn_id: params.turn_id,
|
||||
call_id: params.item_id,
|
||||
started_at_ms: params.started_at_ms,
|
||||
reason: params.reason,
|
||||
permissions: params.permissions.into(),
|
||||
cwd: Some(params.cwd),
|
||||
@@ -6374,8 +6375,9 @@ impl ChatWidget {
|
||||
self.on_guardian_review_notification(
|
||||
notification.review_id,
|
||||
notification.turn_id,
|
||||
notification.started_at_ms,
|
||||
notification.review,
|
||||
/*decision_source*/ None,
|
||||
/*completion*/ None,
|
||||
notification.action,
|
||||
);
|
||||
}
|
||||
@@ -6383,8 +6385,9 @@ impl ChatWidget {
|
||||
self.on_guardian_review_notification(
|
||||
notification.review_id,
|
||||
notification.turn_id,
|
||||
notification.started_at_ms,
|
||||
notification.review,
|
||||
Some(notification.decision_source),
|
||||
Some((notification.completed_at_ms, notification.decision_source)),
|
||||
notification.action,
|
||||
);
|
||||
}
|
||||
@@ -6567,14 +6570,24 @@ impl ChatWidget {
|
||||
&mut self,
|
||||
id: String,
|
||||
turn_id: String,
|
||||
started_at_ms: i64,
|
||||
review: codex_app_server_protocol::GuardianApprovalReview,
|
||||
decision_source: Option<codex_app_server_protocol::AutoReviewDecisionSource>,
|
||||
completion: Option<(i64, codex_app_server_protocol::AutoReviewDecisionSource)>,
|
||||
action: GuardianApprovalReviewAction,
|
||||
) {
|
||||
let (completed_at_ms, decision_source) = match completion {
|
||||
Some((completed_at_ms, decision_source)) => {
|
||||
(Some(completed_at_ms), Some(decision_source))
|
||||
}
|
||||
None => (None, None),
|
||||
};
|
||||
|
||||
self.on_guardian_assessment(GuardianAssessmentEvent {
|
||||
id,
|
||||
target_item_id: None,
|
||||
turn_id,
|
||||
started_at_ms,
|
||||
completed_at_ms,
|
||||
status: match review.status {
|
||||
codex_app_server_protocol::GuardianApprovalReviewStatus::InProgress => {
|
||||
GuardianAssessmentStatus::InProgress
|
||||
|
||||
@@ -55,6 +55,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: Some("approval-1".to_string()),
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
@@ -93,6 +94,7 @@ fn app_server_exec_approval_request_preserves_permissions_context() {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: Some("approval-1".to_string()),
|
||||
reason: None,
|
||||
network_approval_context: Some(codex_app_server_protocol::NetworkApprovalContext {
|
||||
@@ -156,6 +158,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
cwd: cwd.clone(),
|
||||
reason: Some("Select a workspace root".to_string()),
|
||||
permissions: codex_app_server_protocol::RequestPermissionProfile {
|
||||
|
||||
@@ -54,6 +54,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: Some("approval-1".to_string()),
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
|
||||
@@ -6,6 +6,8 @@ fn auto_review_denial_event() -> GuardianAssessmentEvent {
|
||||
id: "auto-review-recent-1".into(),
|
||||
target_item_id: Some("target-auto-review-recent-1".into()),
|
||||
turn_id: "turn-recent-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
user_authorization: Some(GuardianUserAuthorization::Low),
|
||||
@@ -73,6 +75,8 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
id: "guardian-1".into(),
|
||||
target_item_id: Some("guardian-target-1".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -85,6 +89,8 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
id: "guardian-1".into(),
|
||||
target_item_id: Some("guardian-target-1".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
user_authorization: Some(GuardianUserAuthorization::Low),
|
||||
@@ -127,6 +133,8 @@ async fn guardian_approved_exec_renders_approved_request() {
|
||||
id: "thread:child-thread:guardian-1".into(),
|
||||
target_item_id: Some("guardian-approved-target".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Approved,
|
||||
risk_level: Some(GuardianRiskLevel::Low),
|
||||
user_authorization: Some(GuardianUserAuthorization::High),
|
||||
@@ -183,6 +191,8 @@ async fn guardian_approved_request_permissions_renders_request_summary() {
|
||||
id: "guardian-request-permissions".into(),
|
||||
target_item_id: None,
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -205,6 +215,8 @@ async fn guardian_approved_request_permissions_renders_request_summary() {
|
||||
id: "guardian-request-permissions".into(),
|
||||
target_item_id: None,
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Approved,
|
||||
risk_level: Some(GuardianRiskLevel::Low),
|
||||
user_authorization: Some(GuardianUserAuthorization::High),
|
||||
@@ -253,6 +265,8 @@ async fn guardian_timed_out_exec_renders_warning_and_timed_out_request() {
|
||||
id: "guardian-1".into(),
|
||||
target_item_id: Some("guardian-target-1".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -265,6 +279,8 @@ async fn guardian_timed_out_exec_renders_warning_and_timed_out_request() {
|
||||
id: "guardian-1".into(),
|
||||
target_item_id: Some("guardian-target-1".into()),
|
||||
turn_id: "turn-1".into(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::TimedOut,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -315,6 +331,7 @@ async fn app_server_guardian_review_started_sets_review_status() {
|
||||
ItemGuardianApprovalReviewStartedNotification {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
review_id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-target-1".to_string()),
|
||||
review: GuardianApprovalReview {
|
||||
@@ -356,6 +373,7 @@ async fn app_server_guardian_review_denied_renders_denied_request_snapshot() {
|
||||
ItemGuardianApprovalReviewStartedNotification {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
review_id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-target-1".to_string()),
|
||||
review: GuardianApprovalReview {
|
||||
@@ -375,6 +393,8 @@ async fn app_server_guardian_review_denied_renders_denied_request_snapshot() {
|
||||
ItemGuardianApprovalReviewCompletedNotification {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: 1,
|
||||
review_id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-target-1".to_string()),
|
||||
decision_source: AppServerGuardianApprovalReviewDecisionSource::Agent,
|
||||
@@ -431,6 +451,7 @@ async fn app_server_guardian_review_timed_out_renders_timed_out_request_snapshot
|
||||
ItemGuardianApprovalReviewStartedNotification {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
review_id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-target-1".to_string()),
|
||||
review: GuardianApprovalReview {
|
||||
@@ -450,6 +471,8 @@ async fn app_server_guardian_review_timed_out_renders_timed_out_request_snapshot
|
||||
ItemGuardianApprovalReviewCompletedNotification {
|
||||
thread_id: "thread-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: 1,
|
||||
review_id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-target-1".to_string()),
|
||||
decision_source: AppServerGuardianApprovalReviewDecisionSource::Agent,
|
||||
@@ -506,6 +529,8 @@ async fn guardian_parallel_reviews_render_aggregate_status_snapshot() {
|
||||
id: id.to_string(),
|
||||
target_item_id: Some(format!("{id}-target")),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -535,6 +560,8 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-1-target".to_string()),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -550,6 +577,8 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
id: "guardian-2".to_string(),
|
||||
target_item_id: Some("guardian-2-target".to_string()),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
@@ -565,6 +594,8 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
id: "guardian-1".to_string(),
|
||||
target_item_id: Some("guardian-1-target".to_string()),
|
||||
turn_id: "turn-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
completed_at_ms: Some(1),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
user_authorization: Some(GuardianUserAuthorization::Low),
|
||||
|
||||
Reference in New Issue
Block a user