mirror of
https://github.com/openai/codex.git
synced 2026-05-17 01:32:32 +00:00
codex: preserve local apply_patch review shape
Keep local/default apply_patch review payloads from surfacing environment context, while still carrying non-local environment ids through Guardian and app-server review projections. Accept the remote approval snapshot shape emitted by the current TUI features. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -640,11 +640,18 @@ impl From<CoreGuardianAssessmentAction> for GuardianApprovalReviewAction {
|
||||
environment_id,
|
||||
cwd,
|
||||
files,
|
||||
} => Self::ApplyPatch {
|
||||
environment_id: Some(environment_id),
|
||||
cwd,
|
||||
files,
|
||||
},
|
||||
} => {
|
||||
let environment_id = if environment_id.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(environment_id)
|
||||
};
|
||||
Self::ApplyPatch {
|
||||
environment_id,
|
||||
cwd,
|
||||
files,
|
||||
}
|
||||
}
|
||||
CoreGuardianAssessmentAction::NetworkAccess {
|
||||
target,
|
||||
host,
|
||||
|
||||
@@ -2160,6 +2160,51 @@ fn guardian_approval_review_action_round_trips_command_shape() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_approval_review_action_round_trips_apply_patch_environment() {
|
||||
let value = json!({
|
||||
"type": "applyPatch",
|
||||
"environmentId": "remote",
|
||||
"cwd": absolute_path_string("tmp"),
|
||||
"files": [absolute_path_string("tmp/example.txt")],
|
||||
});
|
||||
let action: GuardianApprovalReviewAction =
|
||||
serde_json::from_value(value.clone()).expect("guardian review action");
|
||||
|
||||
assert_eq!(
|
||||
action,
|
||||
GuardianApprovalReviewAction::ApplyPatch {
|
||||
environment_id: Some("remote".to_string()),
|
||||
cwd: absolute_path("tmp"),
|
||||
files: vec![absolute_path("tmp/example.txt")],
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&action).expect("serialize guardian review action"),
|
||||
value
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_approval_review_action_accepts_legacy_apply_patch_environment() {
|
||||
let value = json!({
|
||||
"type": "applyPatch",
|
||||
"cwd": absolute_path_string("tmp"),
|
||||
"files": [absolute_path_string("tmp/example.txt")],
|
||||
});
|
||||
let action: GuardianApprovalReviewAction =
|
||||
serde_json::from_value(value).expect("guardian review action");
|
||||
|
||||
assert_eq!(
|
||||
action,
|
||||
GuardianApprovalReviewAction::ApplyPatch {
|
||||
environment_id: None,
|
||||
cwd: absolute_path("tmp"),
|
||||
files: vec![absolute_path("tmp/example.txt")],
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_requirements_deserializes_legacy_fields() {
|
||||
let requirements: NetworkRequirements = serde_json::from_value(json!({
|
||||
|
||||
@@ -506,16 +506,18 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.note_permission_requested(&conversation_id.to_string())
|
||||
.await;
|
||||
let item_id = event.call_id.clone();
|
||||
let environment_id = (!event.environment_id.is_empty()
|
||||
&& event.environment_id != codex_exec_server::LOCAL_ENVIRONMENT_ID)
|
||||
.then_some(event.environment_id.clone());
|
||||
let cwd = environment_id.as_ref().map(|_| event.cwd.clone());
|
||||
|
||||
let params = FileChangeRequestApprovalParams {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event.turn_id.clone(),
|
||||
item_id: item_id.clone(),
|
||||
started_at_ms: event.started_at_ms,
|
||||
environment_id: (!event.environment_id.is_empty()
|
||||
&& event.environment_id != codex_exec_server::LOCAL_ENVIRONMENT_ID)
|
||||
.then_some(event.environment_id.clone()),
|
||||
cwd: event.cwd.clone(),
|
||||
environment_id,
|
||||
cwd,
|
||||
reason: event.reason.clone(),
|
||||
grant_root: event.grant_root.clone(),
|
||||
};
|
||||
|
||||
@@ -315,13 +315,23 @@ pub(crate) fn guardian_approval_request_to_json(
|
||||
cwd,
|
||||
files,
|
||||
patch,
|
||||
} => Ok(serde_json::json!({
|
||||
"tool": "apply_patch",
|
||||
"environment_id": environment_id,
|
||||
"cwd": cwd,
|
||||
"files": files,
|
||||
"patch": patch,
|
||||
})),
|
||||
} => {
|
||||
let mut value = serde_json::json!({
|
||||
"tool": "apply_patch",
|
||||
"cwd": cwd,
|
||||
"files": files,
|
||||
"patch": patch,
|
||||
});
|
||||
if environment_id != codex_exec_server::LOCAL_ENVIRONMENT_ID
|
||||
&& let Value::Object(object) = &mut value
|
||||
{
|
||||
object.insert(
|
||||
"environment_id".to_string(),
|
||||
Value::String(environment_id.clone()),
|
||||
);
|
||||
}
|
||||
Ok(value)
|
||||
}
|
||||
GuardianApprovalRequest::NetworkAccess {
|
||||
id: _,
|
||||
turn_id: _,
|
||||
@@ -404,7 +414,11 @@ pub(crate) fn guardian_assessment_action(
|
||||
files,
|
||||
..
|
||||
} => GuardianAssessmentAction::ApplyPatch {
|
||||
environment_id: environment_id.clone(),
|
||||
environment_id: if environment_id == codex_exec_server::LOCAL_ENVIRONMENT_ID {
|
||||
String::new()
|
||||
} else {
|
||||
environment_id.clone()
|
||||
},
|
||||
cwd: cwd.clone(),
|
||||
files: files.clone(),
|
||||
},
|
||||
|
||||
@@ -890,7 +890,25 @@ fn guardian_assessment_action_redacts_apply_patch_patch_text() {
|
||||
serde_json::to_value(guardian_assessment_action(&action)).expect("serialize action"),
|
||||
serde_json::json!({
|
||||
"type": "apply_patch",
|
||||
"environment_id": "local",
|
||||
"cwd": cwd,
|
||||
"files": [file],
|
||||
}),
|
||||
);
|
||||
|
||||
let remote_action = GuardianApprovalRequest::ApplyPatch {
|
||||
id: "patch-2".to_string(),
|
||||
environment_id: "remote".to_string(),
|
||||
cwd: cwd.clone(),
|
||||
files: vec![file.clone()],
|
||||
patch: "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+secret\n*** End Patch"
|
||||
.to_string(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(guardian_assessment_action(&remote_action)).expect("serialize action"),
|
||||
serde_json::json!({
|
||||
"type": "apply_patch",
|
||||
"environment_id": "remote",
|
||||
"cwd": cwd,
|
||||
"files": [file],
|
||||
}),
|
||||
|
||||
@@ -147,7 +147,7 @@ pub enum GuardianAssessmentAction {
|
||||
cwd: AbsolutePathBuf,
|
||||
},
|
||||
ApplyPatch {
|
||||
#[serde(default)]
|
||||
#[serde(default, skip_serializing_if = "String::is_empty")]
|
||||
environment_id: String,
|
||||
cwd: AbsolutePathBuf,
|
||||
files: Vec<AbsolutePathBuf>,
|
||||
|
||||
@@ -9,6 +9,7 @@ expression: rendered.as_str()
|
||||
Working directory: /tmp/remote-worktree
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for these files (a)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
Reference in New Issue
Block a user