mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Attach guardian rollouts to feedback uploads
Add guardian review_thread_id plumbing so feedback can resolve the hidden guardian session rollout and attach it alongside the parent rollout in both app-server and local /feedback flows. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
co-authored by
Codex
parent
42e932d7bf
commit
56b7efcb98
@@ -1066,6 +1066,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"reviewThreadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"riskLevel": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -7810,6 +7810,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"reviewThreadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"riskLevel": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -4554,6 +4554,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"reviewThreadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"riskLevel": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
+6
@@ -10,6 +10,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"reviewThreadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"riskLevel": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
+6
@@ -10,6 +10,12 @@
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"reviewThreadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"riskLevel": {
|
||||
"anyOf": [
|
||||
{
|
||||
|
||||
@@ -9,4 +9,4 @@ import type { GuardianRiskLevel } from "./GuardianRiskLevel";
|
||||
* `item/autoApprovalReview/*` notifications. This shape is expected to change
|
||||
* soon.
|
||||
*/
|
||||
export type GuardianApprovalReview = { status: GuardianApprovalReviewStatus, riskScore: number | null, riskLevel: GuardianRiskLevel | null, rationale: string | null, };
|
||||
export type GuardianApprovalReview = { status: GuardianApprovalReviewStatus, reviewThreadId: string | null, riskScore: number | null, riskLevel: GuardianRiskLevel | null, rationale: string | null, };
|
||||
|
||||
@@ -4297,6 +4297,9 @@ impl From<CoreGuardianRiskLevel> for GuardianRiskLevel {
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct GuardianApprovalReview {
|
||||
pub status: GuardianApprovalReviewStatus,
|
||||
#[serde(alias = "review_thread_id")]
|
||||
#[ts(type = "string | null")]
|
||||
pub review_thread_id: Option<String>,
|
||||
#[serde(alias = "risk_score")]
|
||||
#[ts(type = "number | null")]
|
||||
pub risk_score: Option<u8>,
|
||||
@@ -7347,6 +7350,7 @@ mod tests {
|
||||
review,
|
||||
GuardianApprovalReview {
|
||||
status: GuardianApprovalReviewStatus::Denied,
|
||||
review_thread_id: None,
|
||||
risk_score: Some(91),
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
rationale: Some("too risky".to_string()),
|
||||
@@ -7367,6 +7371,7 @@ mod tests {
|
||||
review,
|
||||
GuardianApprovalReview {
|
||||
status: GuardianApprovalReviewStatus::Aborted,
|
||||
review_thread_id: None,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
rationale: None,
|
||||
|
||||
@@ -215,6 +215,9 @@ fn guardian_auto_approval_review_notification(
|
||||
GuardianApprovalReviewStatus::Aborted
|
||||
}
|
||||
},
|
||||
review_thread_id: assessment
|
||||
.review_thread_id
|
||||
.map(|thread_id| thread_id.to_string()),
|
||||
risk_score: assessment.risk_score,
|
||||
risk_level: assessment.risk_level.map(Into::into),
|
||||
rationale: assessment.rationale.clone(),
|
||||
@@ -2803,6 +2806,7 @@ mod tests {
|
||||
&GuardianAssessmentEvent {
|
||||
id: "item-1".to_string(),
|
||||
turn_id: String::new(),
|
||||
review_thread_id: None,
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -2820,6 +2824,7 @@ mod tests {
|
||||
payload.review.status,
|
||||
GuardianApprovalReviewStatus::InProgress
|
||||
);
|
||||
assert_eq!(payload.review.review_thread_id, None);
|
||||
assert_eq!(payload.review.risk_score, None);
|
||||
assert_eq!(payload.review.risk_level, None);
|
||||
assert_eq!(payload.review.rationale, None);
|
||||
@@ -2842,6 +2847,7 @@ mod tests {
|
||||
&GuardianAssessmentEvent {
|
||||
id: "item-2".to_string(),
|
||||
turn_id: "turn-from-assessment".to_string(),
|
||||
review_thread_id: Some(ThreadId::new()),
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(91),
|
||||
risk_level: Some(codex_protocol::protocol::GuardianRiskLevel::High),
|
||||
@@ -2856,6 +2862,7 @@ mod tests {
|
||||
assert_eq!(payload.turn_id, "turn-from-assessment");
|
||||
assert_eq!(payload.target_item_id, "item-2");
|
||||
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Denied);
|
||||
assert!(payload.review.review_thread_id.is_some());
|
||||
assert_eq!(payload.review.risk_score, Some(91));
|
||||
assert_eq!(
|
||||
payload.review.risk_level,
|
||||
@@ -2881,6 +2888,7 @@ mod tests {
|
||||
&GuardianAssessmentEvent {
|
||||
id: "item-3".to_string(),
|
||||
turn_id: "turn-from-assessment".to_string(),
|
||||
review_thread_id: None,
|
||||
status: codex_protocol::protocol::GuardianAssessmentStatus::Aborted,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -2895,6 +2903,7 @@ mod tests {
|
||||
assert_eq!(payload.turn_id, "turn-from-assessment");
|
||||
assert_eq!(payload.target_item_id, "item-3");
|
||||
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Aborted);
|
||||
assert_eq!(payload.review.review_thread_id, None);
|
||||
assert_eq!(payload.review.risk_score, None);
|
||||
assert_eq!(payload.review.risk_level, None);
|
||||
assert_eq!(payload.review.rationale, None);
|
||||
|
||||
@@ -208,6 +208,7 @@ use codex_core::exec_env::create_env;
|
||||
use codex_core::features::FEATURES;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::features::Stage;
|
||||
use codex_core::feedback_rollout_attachment_paths;
|
||||
use codex_core::find_archived_thread_path_by_id_str;
|
||||
use codex_core::find_thread_name_by_id;
|
||||
use codex_core::find_thread_names_by_ids;
|
||||
@@ -6995,14 +6996,21 @@ impl CodexMessageProcessor {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let mut attachment_paths = validated_rollout_path.into_iter().collect::<Vec<_>>();
|
||||
if let Some(extra_log_files) = extra_log_files {
|
||||
attachment_paths.extend(extra_log_files);
|
||||
}
|
||||
|
||||
let session_source = self.thread_manager.session_source();
|
||||
let codex_home = self.config.codex_home.clone();
|
||||
|
||||
let upload_result = tokio::task::spawn_blocking(move || {
|
||||
let mut attachment_paths = if include_logs {
|
||||
feedback_rollout_attachment_paths(
|
||||
codex_home.as_path(),
|
||||
validated_rollout_path.as_deref(),
|
||||
)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
if let Some(extra_log_files) = extra_log_files {
|
||||
attachment_paths.extend(extra_log_files);
|
||||
}
|
||||
snapshot.upload_feedback(
|
||||
&classification,
|
||||
reason.as_deref(),
|
||||
|
||||
@@ -314,6 +314,7 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
|
||||
GuardianAssessmentEvent {
|
||||
id: "command-item-1".to_string(),
|
||||
turn_id: parent_ctx.sub_id.clone(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
|
||||
@@ -26,6 +26,7 @@ use super::prompt::guardian_output_schema;
|
||||
use super::prompt::parse_guardian_assessment;
|
||||
use super::review_session::GuardianReviewSessionOutcome;
|
||||
use super::review_session::GuardianReviewSessionParams;
|
||||
use super::review_session::GuardianReviewSessionRunResult;
|
||||
use super::review_session::build_guardian_review_session_config;
|
||||
|
||||
pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
|
||||
@@ -39,9 +40,16 @@ pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(super) enum GuardianReviewOutcome {
|
||||
Completed(anyhow::Result<GuardianAssessment>),
|
||||
TimedOut,
|
||||
Aborted,
|
||||
Completed {
|
||||
result: anyhow::Result<GuardianAssessment>,
|
||||
review_thread_id: Option<codex_protocol::ThreadId>,
|
||||
},
|
||||
TimedOut {
|
||||
review_thread_id: Option<codex_protocol::ThreadId>,
|
||||
},
|
||||
Aborted {
|
||||
review_thread_id: Option<codex_protocol::ThreadId>,
|
||||
},
|
||||
}
|
||||
|
||||
fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str {
|
||||
@@ -88,6 +96,7 @@ async fn run_guardian_review(
|
||||
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: assessment_id.clone(),
|
||||
turn_id: assessment_turn_id.clone(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -107,6 +116,7 @@ async fn run_guardian_review(
|
||||
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: assessment_id,
|
||||
turn_id: assessment_turn_id,
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Aborted,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -131,32 +141,48 @@ async fn run_guardian_review(
|
||||
)
|
||||
.await
|
||||
}
|
||||
Err(err) => GuardianReviewOutcome::Completed(Err(err.into())),
|
||||
Err(err) => GuardianReviewOutcome::Completed {
|
||||
result: Err(err.into()),
|
||||
review_thread_id: None,
|
||||
},
|
||||
};
|
||||
|
||||
let assessment = match outcome {
|
||||
GuardianReviewOutcome::Completed(Ok(assessment)) => assessment,
|
||||
GuardianReviewOutcome::Completed(Err(err)) => GuardianAssessment {
|
||||
risk_level: GuardianRiskLevel::High,
|
||||
risk_score: 100,
|
||||
rationale: format!("Automatic approval review failed: {err}"),
|
||||
evidence: vec![],
|
||||
},
|
||||
GuardianReviewOutcome::TimedOut => GuardianAssessment {
|
||||
risk_level: GuardianRiskLevel::High,
|
||||
risk_score: 100,
|
||||
rationale:
|
||||
"Automatic approval review timed out while evaluating the requested approval."
|
||||
.to_string(),
|
||||
evidence: vec![],
|
||||
},
|
||||
GuardianReviewOutcome::Aborted => {
|
||||
let (assessment, review_thread_id) = match outcome {
|
||||
GuardianReviewOutcome::Completed {
|
||||
result: Ok(assessment),
|
||||
review_thread_id,
|
||||
} => (assessment, review_thread_id),
|
||||
GuardianReviewOutcome::Completed {
|
||||
result: Err(err),
|
||||
review_thread_id,
|
||||
} => (
|
||||
GuardianAssessment {
|
||||
risk_level: GuardianRiskLevel::High,
|
||||
risk_score: 100,
|
||||
rationale: format!("Automatic approval review failed: {err}"),
|
||||
evidence: vec![],
|
||||
},
|
||||
review_thread_id,
|
||||
),
|
||||
GuardianReviewOutcome::TimedOut { review_thread_id } => (
|
||||
GuardianAssessment {
|
||||
risk_level: GuardianRiskLevel::High,
|
||||
risk_score: 100,
|
||||
rationale:
|
||||
"Automatic approval review timed out while evaluating the requested approval."
|
||||
.to_string(),
|
||||
evidence: vec![],
|
||||
},
|
||||
review_thread_id,
|
||||
),
|
||||
GuardianReviewOutcome::Aborted { review_thread_id } => {
|
||||
session
|
||||
.send_event(
|
||||
turn.as_ref(),
|
||||
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: assessment_id,
|
||||
turn_id: assessment_turn_id,
|
||||
review_thread_id,
|
||||
status: GuardianAssessmentStatus::Aborted,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -193,6 +219,7 @@ async fn run_guardian_review(
|
||||
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: assessment_id,
|
||||
turn_id: assessment_turn_id,
|
||||
review_thread_id,
|
||||
status,
|
||||
risk_score: Some(assessment.risk_score),
|
||||
risk_level: Some(assessment.risk_level),
|
||||
@@ -267,7 +294,12 @@ pub(super) async fn run_guardian_review_session(
|
||||
let live_network_config = match session.services.network_proxy.as_ref() {
|
||||
Some(network_proxy) => match network_proxy.proxy().current_cfg().await {
|
||||
Ok(config) => Some(config),
|
||||
Err(err) => return GuardianReviewOutcome::Completed(Err(err)),
|
||||
Err(err) => {
|
||||
return GuardianReviewOutcome::Completed {
|
||||
result: Err(err),
|
||||
review_thread_id: None,
|
||||
};
|
||||
}
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
@@ -317,7 +349,12 @@ pub(super) async fn run_guardian_review_session(
|
||||
);
|
||||
let guardian_config = match guardian_config {
|
||||
Ok(config) => config,
|
||||
Err(err) => return GuardianReviewOutcome::Completed(Err(err)),
|
||||
Err(err) => {
|
||||
return GuardianReviewOutcome::Completed {
|
||||
result: Err(err),
|
||||
review_thread_id: None,
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
match session
|
||||
@@ -336,15 +373,27 @@ pub(super) async fn run_guardian_review_session(
|
||||
})
|
||||
.await
|
||||
{
|
||||
GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)) => {
|
||||
GuardianReviewOutcome::Completed(parse_guardian_assessment(
|
||||
last_agent_message.as_deref(),
|
||||
))
|
||||
}
|
||||
GuardianReviewSessionOutcome::Completed(Err(err)) => {
|
||||
GuardianReviewOutcome::Completed(Err(err))
|
||||
}
|
||||
GuardianReviewSessionOutcome::TimedOut => GuardianReviewOutcome::TimedOut,
|
||||
GuardianReviewSessionOutcome::Aborted => GuardianReviewOutcome::Aborted,
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome: GuardianReviewSessionOutcome::Completed(Ok(last_agent_message)),
|
||||
} => GuardianReviewOutcome::Completed {
|
||||
result: parse_guardian_assessment(last_agent_message.as_deref()),
|
||||
review_thread_id,
|
||||
},
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome: GuardianReviewSessionOutcome::Completed(Err(err)),
|
||||
} => GuardianReviewOutcome::Completed {
|
||||
result: Err(err),
|
||||
review_thread_id,
|
||||
},
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome: GuardianReviewSessionOutcome::TimedOut,
|
||||
} => GuardianReviewOutcome::TimedOut { review_thread_id },
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome: GuardianReviewSessionOutcome::Aborted,
|
||||
} => GuardianReviewOutcome::Aborted { review_thread_id },
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::anyhow;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
@@ -48,6 +49,11 @@ pub(crate) enum GuardianReviewSessionOutcome {
|
||||
Aborted,
|
||||
}
|
||||
|
||||
pub(crate) struct GuardianReviewSessionRunResult {
|
||||
pub(crate) review_thread_id: Option<ThreadId>,
|
||||
pub(crate) outcome: GuardianReviewSessionOutcome,
|
||||
}
|
||||
|
||||
pub(crate) struct GuardianReviewSessionParams {
|
||||
pub(crate) parent_session: Arc<Session>,
|
||||
pub(crate) parent_turn: Arc<TurnContext>,
|
||||
@@ -236,7 +242,7 @@ impl GuardianReviewSessionManager {
|
||||
pub(crate) async fn run_review(
|
||||
&self,
|
||||
params: GuardianReviewSessionParams,
|
||||
) -> GuardianReviewSessionOutcome {
|
||||
) -> GuardianReviewSessionRunResult {
|
||||
let deadline = tokio::time::Instant::now() + GUARDIAN_REVIEW_TIMEOUT;
|
||||
let next_reuse_key = GuardianReviewSessionReuseKey::from_spawn_config(¶ms.spawn_config);
|
||||
let mut stale_trunk_to_shutdown = None;
|
||||
@@ -273,16 +279,29 @@ impl GuardianReviewSessionManager {
|
||||
{
|
||||
Ok(Ok(review_session)) => Arc::new(review_session),
|
||||
Ok(Err(err)) => {
|
||||
return GuardianReviewSessionOutcome::Completed(Err(err));
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome: GuardianReviewSessionOutcome::Completed(Err(err)),
|
||||
};
|
||||
}
|
||||
Err(outcome) => {
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome,
|
||||
};
|
||||
}
|
||||
Err(outcome) => return outcome,
|
||||
};
|
||||
state.trunk = Some(Arc::clone(&review_session));
|
||||
}
|
||||
|
||||
state.trunk.as_ref().cloned()
|
||||
}
|
||||
Err(outcome) => return outcome,
|
||||
Err(outcome) => {
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome,
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(review_session) = stale_trunk_to_shutdown {
|
||||
@@ -290,9 +309,12 @@ impl GuardianReviewSessionManager {
|
||||
}
|
||||
|
||||
let Some(trunk) = trunk_candidate else {
|
||||
return GuardianReviewSessionOutcome::Completed(Err(anyhow!(
|
||||
"guardian review session was not available after spawn"
|
||||
)));
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome: GuardianReviewSessionOutcome::Completed(Err(anyhow!(
|
||||
"guardian review session was not available after spawn"
|
||||
))),
|
||||
};
|
||||
};
|
||||
|
||||
if trunk.reuse_key != next_reuse_key {
|
||||
@@ -318,18 +340,25 @@ impl GuardianReviewSessionManager {
|
||||
|
||||
let (outcome, keep_review_session) =
|
||||
run_review_on_session(trunk.as_ref(), ¶ms, deadline).await;
|
||||
let review_thread_id = Some(trunk.codex.session.conversation_id);
|
||||
if keep_review_session && matches!(outcome, GuardianReviewSessionOutcome::Completed(_)) {
|
||||
trunk.refresh_last_committed_rollout_items().await;
|
||||
}
|
||||
drop(trunk_guard);
|
||||
|
||||
if keep_review_session {
|
||||
outcome
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome,
|
||||
}
|
||||
} else {
|
||||
if let Some(review_session) = self.remove_trunk_if_current(&trunk).await {
|
||||
review_session.shutdown_in_background();
|
||||
}
|
||||
outcome
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -407,7 +436,7 @@ impl GuardianReviewSessionManager {
|
||||
reuse_key: GuardianReviewSessionReuseKey,
|
||||
deadline: tokio::time::Instant,
|
||||
initial_history: Option<InitialHistory>,
|
||||
) -> GuardianReviewSessionOutcome {
|
||||
) -> GuardianReviewSessionRunResult {
|
||||
let spawn_cancel_token = CancellationToken::new();
|
||||
let mut fork_config = params.spawn_config.clone();
|
||||
fork_config.ephemeral = true;
|
||||
@@ -426,8 +455,18 @@ impl GuardianReviewSessionManager {
|
||||
.await
|
||||
{
|
||||
Ok(Ok(review_session)) => Arc::new(review_session),
|
||||
Ok(Err(err)) => return GuardianReviewSessionOutcome::Completed(Err(err)),
|
||||
Err(outcome) => return outcome,
|
||||
Ok(Err(err)) => {
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome: GuardianReviewSessionOutcome::Completed(Err(err)),
|
||||
};
|
||||
}
|
||||
Err(outcome) => {
|
||||
return GuardianReviewSessionRunResult {
|
||||
review_thread_id: None,
|
||||
outcome,
|
||||
};
|
||||
}
|
||||
};
|
||||
self.register_active_ephemeral(Arc::clone(&review_session))
|
||||
.await;
|
||||
@@ -435,11 +474,15 @@ impl GuardianReviewSessionManager {
|
||||
EphemeralReviewCleanup::new(Arc::clone(&self.state), Arc::clone(&review_session));
|
||||
|
||||
let (outcome, _) = run_review_on_session(review_session.as_ref(), ¶ms, deadline).await;
|
||||
let review_thread_id = Some(review_session.codex.session.conversation_id);
|
||||
if let Some(review_session) = self.take_active_ephemeral(&review_session).await {
|
||||
cleanup.disarm();
|
||||
review_session.shutdown_in_background();
|
||||
}
|
||||
outcome
|
||||
GuardianReviewSessionRunResult {
|
||||
review_thread_id,
|
||||
outcome,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -555,7 +555,11 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
let GuardianReviewOutcome::Completed(Ok(assessment)) = outcome else {
|
||||
let GuardianReviewOutcome::Completed {
|
||||
result: Ok(assessment),
|
||||
..
|
||||
} = outcome
|
||||
else {
|
||||
panic!("expected guardian assessment");
|
||||
};
|
||||
assert_eq!(assessment.risk_score, 35);
|
||||
@@ -659,10 +663,18 @@ async fn guardian_reuses_prompt_cache_key_and_appends_prior_reviews() -> anyhow:
|
||||
)
|
||||
.await;
|
||||
|
||||
let GuardianReviewOutcome::Completed(Ok(first_assessment)) = first_outcome else {
|
||||
let GuardianReviewOutcome::Completed {
|
||||
result: Ok(first_assessment),
|
||||
..
|
||||
} = first_outcome
|
||||
else {
|
||||
panic!("expected first guardian assessment");
|
||||
};
|
||||
let GuardianReviewOutcome::Completed(Ok(second_assessment)) = second_outcome else {
|
||||
let GuardianReviewOutcome::Completed {
|
||||
result: Ok(second_assessment),
|
||||
..
|
||||
} = second_outcome
|
||||
else {
|
||||
panic!("expected second guardian assessment");
|
||||
};
|
||||
assert_eq!(first_assessment.risk_score, 5);
|
||||
|
||||
@@ -132,6 +132,7 @@ pub use rollout::RolloutRecorderParams;
|
||||
pub use rollout::SESSIONS_SUBDIR;
|
||||
pub use rollout::SessionMeta;
|
||||
pub use rollout::append_thread_name;
|
||||
pub use rollout::feedback_rollout_attachment_paths;
|
||||
pub use rollout::find_archived_thread_path_by_id_str;
|
||||
#[deprecated(note = "use find_thread_path_by_id_str")]
|
||||
pub use rollout::find_conversation_path_by_id_str;
|
||||
|
||||
@@ -0,0 +1,304 @@
|
||||
use std::collections::HashSet;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::io::BufRead;
|
||||
use std::io::BufReader;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use tracing::warn;
|
||||
|
||||
use super::ARCHIVED_SESSIONS_SUBDIR;
|
||||
use super::SESSIONS_SUBDIR;
|
||||
|
||||
pub fn feedback_rollout_attachment_paths(
|
||||
codex_home: &Path,
|
||||
rollout_path: Option<&Path>,
|
||||
) -> Vec<PathBuf> {
|
||||
let Some(rollout_path) = rollout_path else {
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
let mut attachment_paths = Vec::new();
|
||||
let mut seen_paths = HashSet::new();
|
||||
push_existing_unique_path(
|
||||
&mut attachment_paths,
|
||||
&mut seen_paths,
|
||||
rollout_path.to_path_buf(),
|
||||
);
|
||||
|
||||
let guardian_thread_ids = match guardian_review_thread_ids(rollout_path) {
|
||||
Ok(thread_ids) => thread_ids,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %rollout_path.display(),
|
||||
error = %err,
|
||||
"failed to read guardian review thread ids from rollout"
|
||||
);
|
||||
return attachment_paths;
|
||||
}
|
||||
};
|
||||
|
||||
for guardian_thread_id in guardian_thread_ids {
|
||||
let Some(guardian_rollout_path) =
|
||||
find_rollout_path_by_thread_id(codex_home, guardian_thread_id)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
push_existing_unique_path(
|
||||
&mut attachment_paths,
|
||||
&mut seen_paths,
|
||||
guardian_rollout_path,
|
||||
);
|
||||
}
|
||||
|
||||
attachment_paths
|
||||
}
|
||||
|
||||
fn guardian_review_thread_ids(rollout_path: &Path) -> io::Result<Vec<ThreadId>> {
|
||||
let file = fs::File::open(rollout_path)?;
|
||||
let reader = BufReader::new(file);
|
||||
let mut thread_ids = Vec::new();
|
||||
let mut seen_thread_ids = HashSet::new();
|
||||
|
||||
for line in reader.lines() {
|
||||
let line = line?;
|
||||
let rollout_line = match serde_json::from_str::<RolloutLine>(&line) {
|
||||
Ok(rollout_line) => rollout_line,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %rollout_path.display(),
|
||||
error = %err,
|
||||
"failed to parse rollout line while collecting guardian review thread ids"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if let RolloutItem::EventMsg(EventMsg::GuardianAssessment(assessment)) = rollout_line.item
|
||||
&& let Some(review_thread_id) = assessment.review_thread_id
|
||||
&& seen_thread_ids.insert(review_thread_id)
|
||||
{
|
||||
thread_ids.push(review_thread_id);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(thread_ids)
|
||||
}
|
||||
|
||||
fn find_rollout_path_by_thread_id(codex_home: &Path, thread_id: ThreadId) -> Option<PathBuf> {
|
||||
let thread_id = thread_id.to_string();
|
||||
find_rollout_path_by_thread_id_in_subdir(codex_home, SESSIONS_SUBDIR, &thread_id).or_else(
|
||||
|| {
|
||||
find_rollout_path_by_thread_id_in_subdir(
|
||||
codex_home,
|
||||
ARCHIVED_SESSIONS_SUBDIR,
|
||||
&thread_id,
|
||||
)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
fn find_rollout_path_by_thread_id_in_subdir(
|
||||
codex_home: &Path,
|
||||
subdir: &str,
|
||||
thread_id: &str,
|
||||
) -> Option<PathBuf> {
|
||||
let root = codex_home.join(subdir);
|
||||
if !root.exists() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let expected_suffix = format!("-{thread_id}.jsonl");
|
||||
let mut dirs = vec![root];
|
||||
while let Some(dir) = dirs.pop() {
|
||||
let entries = match fs::read_dir(&dir) {
|
||||
Ok(entries) => entries,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %dir.display(),
|
||||
error = %err,
|
||||
"failed to scan rollout directory while resolving guardian rollout"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
for entry in entries {
|
||||
let entry = match entry {
|
||||
Ok(entry) => entry,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %dir.display(),
|
||||
error = %err,
|
||||
"failed to read rollout directory entry while resolving guardian rollout"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let path = entry.path();
|
||||
let file_type = match entry.file_type() {
|
||||
Ok(file_type) => file_type,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
path = %path.display(),
|
||||
error = %err,
|
||||
"failed to inspect rollout directory entry while resolving guardian rollout"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
if file_type.is_dir() {
|
||||
dirs.push(path);
|
||||
continue;
|
||||
}
|
||||
|
||||
if file_type.is_file()
|
||||
&& path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.ends_with(&expected_suffix))
|
||||
{
|
||||
return Some(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn push_existing_unique_path(
|
||||
attachment_paths: &mut Vec<PathBuf>,
|
||||
seen_paths: &mut HashSet<PathBuf>,
|
||||
path: PathBuf,
|
||||
) {
|
||||
if path.exists() && seen_paths.insert(path.clone()) {
|
||||
attachment_paths.push(path);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::fs;
|
||||
|
||||
use codex_protocol::protocol::GuardianAssessmentEvent;
|
||||
use codex_protocol::protocol::GuardianAssessmentStatus;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn feedback_rollout_attachment_paths_include_guardian_rollouts() {
|
||||
let tempdir = tempdir().expect("tempdir");
|
||||
let codex_home = tempdir.path();
|
||||
|
||||
let parent_thread_id = ThreadId::new();
|
||||
let guardian_thread_id = ThreadId::new();
|
||||
let parent_rollout_path = write_rollout(
|
||||
codex_home,
|
||||
SESSIONS_SUBDIR,
|
||||
parent_thread_id,
|
||||
&[
|
||||
GuardianAssessmentEvent {
|
||||
id: "assessment-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: Some(guardian_thread_id),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(100),
|
||||
risk_level: None,
|
||||
rationale: Some("too risky".to_string()),
|
||||
action: None,
|
||||
},
|
||||
GuardianAssessmentEvent {
|
||||
id: "assessment-2".to_string(),
|
||||
turn_id: "turn-2".to_string(),
|
||||
review_thread_id: Some(guardian_thread_id),
|
||||
status: GuardianAssessmentStatus::Approved,
|
||||
risk_score: Some(0),
|
||||
risk_level: None,
|
||||
rationale: Some("safe".to_string()),
|
||||
action: None,
|
||||
},
|
||||
],
|
||||
);
|
||||
let guardian_rollout_path = write_rollout(
|
||||
codex_home,
|
||||
ARCHIVED_SESSIONS_SUBDIR,
|
||||
guardian_thread_id,
|
||||
&[],
|
||||
);
|
||||
|
||||
let attachment_paths =
|
||||
feedback_rollout_attachment_paths(codex_home, Some(parent_rollout_path.as_path()));
|
||||
|
||||
assert_eq!(
|
||||
attachment_paths,
|
||||
vec![parent_rollout_path, guardian_rollout_path]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feedback_rollout_attachment_paths_ignore_missing_guardian_rollouts() {
|
||||
let tempdir = tempdir().expect("tempdir");
|
||||
let codex_home = tempdir.path();
|
||||
|
||||
let parent_thread_id = ThreadId::new();
|
||||
let missing_guardian_thread_id = ThreadId::new();
|
||||
let parent_rollout_path = write_rollout(
|
||||
codex_home,
|
||||
SESSIONS_SUBDIR,
|
||||
parent_thread_id,
|
||||
&[GuardianAssessmentEvent {
|
||||
id: "assessment-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: Some(missing_guardian_thread_id),
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(100),
|
||||
risk_level: None,
|
||||
rationale: Some("too risky".to_string()),
|
||||
action: None,
|
||||
}],
|
||||
);
|
||||
|
||||
let attachment_paths =
|
||||
feedback_rollout_attachment_paths(codex_home, Some(parent_rollout_path.as_path()));
|
||||
|
||||
assert_eq!(attachment_paths, vec![parent_rollout_path]);
|
||||
}
|
||||
|
||||
fn write_rollout(
|
||||
codex_home: &Path,
|
||||
subdir: &str,
|
||||
thread_id: ThreadId,
|
||||
assessments: &[GuardianAssessmentEvent],
|
||||
) -> PathBuf {
|
||||
let dir = codex_home.join(subdir).join("2026").join("03").join("18");
|
||||
fs::create_dir_all(&dir).expect("create rollout dir");
|
||||
let path = dir.join(format!("rollout-2026-03-18T12-00-00-{thread_id}.jsonl"));
|
||||
|
||||
let contents = assessments
|
||||
.iter()
|
||||
.map(|assessment| {
|
||||
serde_json::to_string(&RolloutLine {
|
||||
timestamp: "2026-03-18T12:00:00.000Z".to_string(),
|
||||
item: RolloutItem::EventMsg(EventMsg::GuardianAssessment(assessment.clone())),
|
||||
})
|
||||
.expect("serialize rollout line")
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
|
||||
if contents.is_empty() {
|
||||
fs::write(&path, "").expect("write rollout");
|
||||
} else {
|
||||
fs::write(&path, format!("{contents}\n")).expect("write rollout");
|
||||
}
|
||||
|
||||
path
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,7 @@ pub const INTERACTIVE_SESSION_SOURCES: &[SessionSource] =
|
||||
&[SessionSource::Cli, SessionSource::VSCode];
|
||||
|
||||
pub(crate) mod error;
|
||||
mod feedback;
|
||||
pub mod list;
|
||||
pub(crate) mod metadata;
|
||||
pub(crate) mod policy;
|
||||
@@ -17,6 +18,7 @@ pub(crate) mod truncation;
|
||||
|
||||
pub use codex_protocol::protocol::SessionMeta;
|
||||
pub(crate) use error::map_session_init_error;
|
||||
pub use feedback::feedback_rollout_attachment_paths;
|
||||
pub use list::find_archived_thread_path_by_id_str;
|
||||
pub use list::find_thread_path_by_id_str;
|
||||
#[deprecated(note = "use find_thread_path_by_id_str")]
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::ThreadId;
|
||||
use crate::mcp::RequestId;
|
||||
use crate::models::MacOsSeatbeltProfileExtensions;
|
||||
use crate::models::PermissionProfile;
|
||||
@@ -122,6 +123,10 @@ pub struct GuardianAssessmentEvent {
|
||||
/// Uses `#[serde(default)]` for backwards compatibility.
|
||||
#[serde(default)]
|
||||
pub turn_id: String,
|
||||
/// Hidden guardian review thread that evaluated this request, when known.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub review_thread_id: Option<ThreadId>,
|
||||
pub status: GuardianAssessmentStatus,
|
||||
/// Numeric risk score from 0-100. Omitted while the assessment is in progress.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -45,11 +45,11 @@ pub(crate) enum FeedbackAudience {
|
||||
}
|
||||
|
||||
/// Minimal input overlay to collect an optional feedback note, then upload
|
||||
/// both logs and rollout with classification + metadata.
|
||||
/// logs and rollout attachments with classification + metadata.
|
||||
pub(crate) struct FeedbackNoteView {
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
rollout_path: Option<PathBuf>,
|
||||
attachment_paths: Vec<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
feedback_audience: FeedbackAudience,
|
||||
@@ -64,7 +64,7 @@ impl FeedbackNoteView {
|
||||
pub(crate) fn new(
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
rollout_path: Option<PathBuf>,
|
||||
attachment_paths: Vec<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
feedback_audience: FeedbackAudience,
|
||||
@@ -72,7 +72,7 @@ impl FeedbackNoteView {
|
||||
Self {
|
||||
category,
|
||||
snapshot,
|
||||
rollout_path,
|
||||
attachment_paths,
|
||||
app_event_tx,
|
||||
include_logs,
|
||||
feedback_audience,
|
||||
@@ -89,11 +89,6 @@ impl FeedbackNoteView {
|
||||
} else {
|
||||
Some(note.as_str())
|
||||
};
|
||||
let attachment_paths = if self.include_logs {
|
||||
self.rollout_path.iter().cloned().collect::<Vec<_>>()
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let classification = feedback_classification(self.category);
|
||||
|
||||
let mut thread_id = self.snapshot.thread_id.clone();
|
||||
@@ -102,7 +97,7 @@ impl FeedbackNoteView {
|
||||
classification,
|
||||
reason_opt,
|
||||
self.include_logs,
|
||||
&attachment_paths,
|
||||
&self.attachment_paths,
|
||||
Some(SessionSource::Cli),
|
||||
/*logs_override*/ None,
|
||||
);
|
||||
@@ -501,7 +496,7 @@ fn make_feedback_item(
|
||||
pub(crate) fn feedback_upload_consent_params(
|
||||
app_event_tx: AppEventSender,
|
||||
category: FeedbackCategory,
|
||||
rollout_path: Option<std::path::PathBuf>,
|
||||
attachment_paths: Vec<std::path::PathBuf>,
|
||||
feedback_diagnostics: &FeedbackDiagnostics,
|
||||
) -> super::SelectionViewParams {
|
||||
use super::popup_consts::standard_popup_hint_line;
|
||||
@@ -534,10 +529,10 @@ pub(crate) fn feedback_upload_consent_params(
|
||||
Line::from("The following files will be sent:".dim()).into(),
|
||||
Line::from(vec![" • ".into(), "codex-logs.log".into()]).into(),
|
||||
];
|
||||
if let Some(path) = rollout_path.as_deref()
|
||||
&& let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string())
|
||||
{
|
||||
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
|
||||
for path in attachment_paths {
|
||||
if let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) {
|
||||
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
|
||||
}
|
||||
}
|
||||
if !feedback_diagnostics.is_empty() {
|
||||
header_lines.push(
|
||||
@@ -632,7 +627,7 @@ mod tests {
|
||||
FeedbackNoteView::new(
|
||||
category,
|
||||
snapshot,
|
||||
None,
|
||||
Vec::new(),
|
||||
tx,
|
||||
true,
|
||||
FeedbackAudience::External,
|
||||
@@ -695,7 +690,7 @@ mod tests {
|
||||
let view = FeedbackNoteView::new(
|
||||
FeedbackCategory::Bug,
|
||||
snapshot,
|
||||
None,
|
||||
Vec::new(),
|
||||
tx,
|
||||
false,
|
||||
FeedbackAudience::External,
|
||||
|
||||
@@ -62,6 +62,7 @@ use codex_core::config::types::WindowsSandboxModeToml;
|
||||
use codex_core::config_loader::ConfigLayerStackOrdering;
|
||||
use codex_core::features::FEATURES;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::feedback_rollout_attachment_paths;
|
||||
use codex_core::find_thread_name_by_id;
|
||||
use codex_core::git_info::current_branch_name;
|
||||
use codex_core::git_info::get_git_repo_root;
|
||||
@@ -1518,15 +1519,18 @@ impl ChatWidget {
|
||||
include_logs: bool,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
) {
|
||||
let rollout = if include_logs {
|
||||
self.current_rollout_path.clone()
|
||||
let attachment_paths = if include_logs {
|
||||
feedback_rollout_attachment_paths(
|
||||
self.config.codex_home.as_path(),
|
||||
self.current_rollout_path.as_deref(),
|
||||
)
|
||||
} else {
|
||||
None
|
||||
Vec::new()
|
||||
};
|
||||
let view = crate::bottom_pane::FeedbackNoteView::new(
|
||||
category,
|
||||
snapshot,
|
||||
rollout,
|
||||
attachment_paths,
|
||||
self.app_event_tx.clone(),
|
||||
include_logs,
|
||||
self.feedback_audience,
|
||||
@@ -1553,7 +1557,10 @@ impl ChatWidget {
|
||||
let params = crate::bottom_pane::feedback_upload_consent_params(
|
||||
self.app_event_tx.clone(),
|
||||
category,
|
||||
self.current_rollout_path.clone(),
|
||||
feedback_rollout_attachment_paths(
|
||||
self.config.codex_home.as_path(),
|
||||
self.current_rollout_path.as_deref(),
|
||||
),
|
||||
snapshot.feedback_diagnostics(),
|
||||
);
|
||||
self.bottom_pane.show_selection_view(params);
|
||||
|
||||
@@ -8136,7 +8136,7 @@ async fn feedback_upload_consent_popup_snapshot() {
|
||||
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
|
||||
chat.app_event_tx.clone(),
|
||||
crate::app_event::FeedbackCategory::Bug,
|
||||
chat.current_rollout_path.clone(),
|
||||
chat.current_rollout_path.clone().into_iter().collect(),
|
||||
&codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![
|
||||
codex_feedback::feedback_diagnostics::FeedbackDiagnostic {
|
||||
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
|
||||
@@ -8156,7 +8156,7 @@ async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_fi
|
||||
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
|
||||
chat.app_event_tx.clone(),
|
||||
crate::app_event::FeedbackCategory::GoodResult,
|
||||
chat.current_rollout_path.clone(),
|
||||
chat.current_rollout_path.clone().into_iter().collect(),
|
||||
&codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![
|
||||
codex_feedback::feedback_diagnostics::FeedbackDiagnostic {
|
||||
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
|
||||
@@ -9500,6 +9500,7 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -9518,6 +9519,7 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(96),
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
@@ -9561,6 +9563,7 @@ async fn guardian_approved_exec_renders_approved_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "thread:child-thread:guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Approved,
|
||||
risk_score: Some(14),
|
||||
risk_level: Some(GuardianRiskLevel::Low),
|
||||
@@ -9708,6 +9711,7 @@ async fn guardian_parallel_reviews_render_aggregate_status_snapshot() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: id.to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -9737,6 +9741,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -9752,6 +9757,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-2".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -9767,6 +9773,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(92),
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
|
||||
@@ -45,11 +45,11 @@ pub(crate) enum FeedbackAudience {
|
||||
}
|
||||
|
||||
/// Minimal input overlay to collect an optional feedback note, then upload
|
||||
/// both logs and rollout with classification + metadata.
|
||||
/// logs and rollout attachments with classification + metadata.
|
||||
pub(crate) struct FeedbackNoteView {
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
rollout_path: Option<PathBuf>,
|
||||
attachment_paths: Vec<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
feedback_audience: FeedbackAudience,
|
||||
@@ -64,7 +64,7 @@ impl FeedbackNoteView {
|
||||
pub(crate) fn new(
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
rollout_path: Option<PathBuf>,
|
||||
attachment_paths: Vec<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
feedback_audience: FeedbackAudience,
|
||||
@@ -72,7 +72,7 @@ impl FeedbackNoteView {
|
||||
Self {
|
||||
category,
|
||||
snapshot,
|
||||
rollout_path,
|
||||
attachment_paths,
|
||||
app_event_tx,
|
||||
include_logs,
|
||||
feedback_audience,
|
||||
@@ -89,11 +89,6 @@ impl FeedbackNoteView {
|
||||
} else {
|
||||
Some(note.as_str())
|
||||
};
|
||||
let attachment_paths = if self.include_logs {
|
||||
self.rollout_path.iter().cloned().collect::<Vec<_>>()
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let classification = feedback_classification(self.category);
|
||||
|
||||
let mut thread_id = self.snapshot.thread_id.clone();
|
||||
@@ -102,7 +97,7 @@ impl FeedbackNoteView {
|
||||
classification,
|
||||
reason_opt,
|
||||
self.include_logs,
|
||||
&attachment_paths,
|
||||
&self.attachment_paths,
|
||||
Some(SessionSource::Cli),
|
||||
/*logs_override*/ None,
|
||||
);
|
||||
@@ -501,7 +496,7 @@ fn make_feedback_item(
|
||||
pub(crate) fn feedback_upload_consent_params(
|
||||
app_event_tx: AppEventSender,
|
||||
category: FeedbackCategory,
|
||||
rollout_path: Option<std::path::PathBuf>,
|
||||
attachment_paths: Vec<std::path::PathBuf>,
|
||||
feedback_diagnostics: &FeedbackDiagnostics,
|
||||
) -> super::SelectionViewParams {
|
||||
use super::popup_consts::standard_popup_hint_line;
|
||||
@@ -534,10 +529,10 @@ pub(crate) fn feedback_upload_consent_params(
|
||||
Line::from("The following files will be sent:".dim()).into(),
|
||||
Line::from(vec![" • ".into(), "codex-logs.log".into()]).into(),
|
||||
];
|
||||
if let Some(path) = rollout_path.as_deref()
|
||||
&& let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string())
|
||||
{
|
||||
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
|
||||
for path in attachment_paths {
|
||||
if let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string()) {
|
||||
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
|
||||
}
|
||||
}
|
||||
if !feedback_diagnostics.is_empty() {
|
||||
header_lines.push(
|
||||
@@ -632,7 +627,7 @@ mod tests {
|
||||
FeedbackNoteView::new(
|
||||
category,
|
||||
snapshot,
|
||||
None,
|
||||
Vec::new(),
|
||||
tx,
|
||||
true,
|
||||
FeedbackAudience::External,
|
||||
@@ -695,7 +690,7 @@ mod tests {
|
||||
let view = FeedbackNoteView::new(
|
||||
FeedbackCategory::Bug,
|
||||
snapshot,
|
||||
None,
|
||||
Vec::new(),
|
||||
tx,
|
||||
false,
|
||||
FeedbackAudience::External,
|
||||
|
||||
@@ -87,6 +87,7 @@ use codex_core::config::types::WindowsSandboxModeToml;
|
||||
use codex_core::config_loader::ConfigLayerStackOrdering;
|
||||
use codex_core::features::FEATURES;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::feedback_rollout_attachment_paths;
|
||||
use codex_core::find_thread_name_by_id;
|
||||
use codex_core::git_info::current_branch_name;
|
||||
use codex_core::git_info::get_git_repo_root;
|
||||
@@ -1905,15 +1906,18 @@ impl ChatWidget {
|
||||
include_logs: bool,
|
||||
snapshot: codex_feedback::FeedbackSnapshot,
|
||||
) {
|
||||
let rollout = if include_logs {
|
||||
self.current_rollout_path.clone()
|
||||
let attachment_paths = if include_logs {
|
||||
feedback_rollout_attachment_paths(
|
||||
self.config.codex_home.as_path(),
|
||||
self.current_rollout_path.as_deref(),
|
||||
)
|
||||
} else {
|
||||
None
|
||||
Vec::new()
|
||||
};
|
||||
let view = crate::bottom_pane::FeedbackNoteView::new(
|
||||
category,
|
||||
snapshot,
|
||||
rollout,
|
||||
attachment_paths,
|
||||
self.app_event_tx.clone(),
|
||||
include_logs,
|
||||
self.feedback_audience,
|
||||
@@ -1933,7 +1937,10 @@ impl ChatWidget {
|
||||
let params = crate::bottom_pane::feedback_upload_consent_params(
|
||||
self.app_event_tx.clone(),
|
||||
category,
|
||||
self.current_rollout_path.clone(),
|
||||
feedback_rollout_attachment_paths(
|
||||
self.config.codex_home.as_path(),
|
||||
self.current_rollout_path.as_deref(),
|
||||
),
|
||||
snapshot.feedback_diagnostics(),
|
||||
);
|
||||
self.bottom_pane.show_selection_view(params);
|
||||
@@ -6246,6 +6253,10 @@ impl ChatWidget {
|
||||
self.on_guardian_assessment(GuardianAssessmentEvent {
|
||||
id,
|
||||
turn_id,
|
||||
review_thread_id: review
|
||||
.review_thread_id
|
||||
.as_deref()
|
||||
.and_then(|thread_id| ThreadId::from_string(thread_id).ok()),
|
||||
status: match review.status {
|
||||
codex_app_server_protocol::GuardianApprovalReviewStatus::InProgress => {
|
||||
GuardianAssessmentStatus::InProgress
|
||||
|
||||
@@ -8734,7 +8734,7 @@ async fn feedback_upload_consent_popup_snapshot() {
|
||||
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
|
||||
chat.app_event_tx.clone(),
|
||||
crate::app_event::FeedbackCategory::Bug,
|
||||
chat.current_rollout_path.clone(),
|
||||
chat.current_rollout_path.clone().into_iter().collect(),
|
||||
&codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![
|
||||
codex_feedback::feedback_diagnostics::FeedbackDiagnostic {
|
||||
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
|
||||
@@ -8754,7 +8754,7 @@ async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_fi
|
||||
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
|
||||
chat.app_event_tx.clone(),
|
||||
crate::app_event::FeedbackCategory::GoodResult,
|
||||
chat.current_rollout_path.clone(),
|
||||
chat.current_rollout_path.clone().into_iter().collect(),
|
||||
&codex_feedback::feedback_diagnostics::FeedbackDiagnostics::new(vec![
|
||||
codex_feedback::feedback_diagnostics::FeedbackDiagnostic {
|
||||
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
|
||||
@@ -10113,6 +10113,7 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -10131,6 +10132,7 @@ async fn guardian_denied_exec_renders_warning_and_denied_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(96),
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
@@ -10174,6 +10176,7 @@ async fn guardian_approved_exec_renders_approved_request() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "thread:child-thread:guardian-1".into(),
|
||||
turn_id: "turn-1".into(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Approved,
|
||||
risk_score: Some(14),
|
||||
risk_level: Some(GuardianRiskLevel::Low),
|
||||
@@ -10226,6 +10229,7 @@ async fn app_server_guardian_review_started_sets_review_status() {
|
||||
target_item_id: "guardian-1".to_string(),
|
||||
review: GuardianApprovalReview {
|
||||
status: GuardianApprovalReviewStatus::InProgress,
|
||||
review_thread_id: None,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
rationale: None,
|
||||
@@ -10264,6 +10268,7 @@ async fn app_server_guardian_review_denied_renders_denied_request_snapshot() {
|
||||
target_item_id: "guardian-1".to_string(),
|
||||
review: GuardianApprovalReview {
|
||||
status: GuardianApprovalReviewStatus::InProgress,
|
||||
review_thread_id: None,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
rationale: None,
|
||||
@@ -10282,6 +10287,7 @@ async fn app_server_guardian_review_denied_renders_denied_request_snapshot() {
|
||||
target_item_id: "guardian-1".to_string(),
|
||||
review: GuardianApprovalReview {
|
||||
status: GuardianApprovalReviewStatus::Denied,
|
||||
review_thread_id: None,
|
||||
risk_score: Some(96),
|
||||
risk_level: Some(AppServerGuardianRiskLevel::High),
|
||||
rationale: Some("Would exfiltrate local source code.".to_string()),
|
||||
@@ -10428,6 +10434,7 @@ async fn guardian_parallel_reviews_render_aggregate_status_snapshot() {
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: id.to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -10457,6 +10464,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -10472,6 +10480,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-2".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::InProgress,
|
||||
risk_score: None,
|
||||
risk_level: None,
|
||||
@@ -10487,6 +10496,7 @@ async fn guardian_parallel_reviews_keep_remaining_review_visible_after_denial()
|
||||
msg: EventMsg::GuardianAssessment(GuardianAssessmentEvent {
|
||||
id: "guardian-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
review_thread_id: None,
|
||||
status: GuardianAssessmentStatus::Denied,
|
||||
risk_score: Some(92),
|
||||
risk_level: Some(GuardianRiskLevel::High),
|
||||
|
||||
Reference in New Issue
Block a user