toggle via include_permission_instructions

This commit is contained in:
Dylan Hurd
2026-06-12 13:48:46 -07:00
parent 21fbace780
commit 5da72203b1
8 changed files with 194 additions and 5 deletions
+19
View File
@@ -1344,6 +1344,25 @@ impl Config {
}
}
/// Update the effective cwd while keeping the implicit runtime workspace
/// root attached to it. Explicit runtime roots remain stable.
pub fn set_cwd_retargeting_implicit_workspace_root(&mut self, cwd: AbsolutePathBuf) {
let previous_cwd = std::mem::replace(&mut self.cwd, cwd.clone());
if self.workspace_roots_explicit || !self.workspace_roots.contains(&previous_cwd) {
return;
}
let previous_workspace_roots = std::mem::take(&mut self.workspace_roots);
self.workspace_roots.push(cwd);
for root in previous_workspace_roots {
if root != previous_cwd && !self.workspace_roots.contains(&root) {
self.workspace_roots.push(root);
}
}
self.permissions
.set_workspace_roots(self.workspace_roots.clone());
}
pub fn legacy_sandbox_policy(&self) -> SandboxPolicy {
self.permissions.legacy_sandbox_policy(self.cwd.as_path())
}
+16
View File
@@ -60,6 +60,22 @@ fn test_writable_roots_constraint() {
));
}
#[test]
fn workspace_permission_profile_allows_absolute_patch_inside_cwd() {
let tmp = TempDir::new().unwrap();
let cwd = tmp.path().abs();
let inside_path = cwd.join("inside.txt");
let action = ApplyPatchAction::new_add_for_test(&inside_path, "".to_string());
let permission_profile = PermissionProfile::workspace_write();
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
assert!(is_write_patch_constrained_to_writable_paths(
&action,
&file_system_sandbox_policy,
&cwd,
));
}
#[test]
fn external_sandbox_auto_approves_in_on_request() {
let tmp = TempDir::new().unwrap();
+6 -2
View File
@@ -2993,8 +2993,12 @@ impl Session {
.agent_control
.format_environment_context_subagents(self.thread_id)
.await;
let approved_command_prefixes =
format_allow_prefixes(self.services.exec_policy.current().get_allowed_prefixes());
let approved_command_prefixes = if turn_context.config.include_permissions_instructions
{
format_allow_prefixes(self.services.exec_policy.current().get_allowed_prefixes())
} else {
None
};
contextual_user_sections.push(
crate::context::EnvironmentContext::from_turn_context(turn_context, shell.as_ref())
.with_subagents(subagents)
@@ -267,7 +267,7 @@ pub(crate) fn apply_spawn_agent_runtime_overrides(
config.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone();
#[allow(deprecated)]
let turn_cwd = turn.cwd.clone();
config.cwd = turn_cwd;
config.set_cwd_retargeting_implicit_workspace_root(turn_cwd);
config
.permissions
.set_permission_profile(turn.permission_profile())
@@ -4506,7 +4506,7 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
expected.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone();
#[allow(deprecated)]
{
expected.cwd = turn.cwd.clone();
expected.set_cwd_retargeting_implicit_workspace_root(turn.cwd.clone());
}
expected
.permissions
@@ -4520,6 +4520,40 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
assert_eq!(config, expected);
}
#[tokio::test]
async fn build_agent_spawn_config_retargets_implicit_workspace_root_with_turn_cwd() {
let (_session, mut turn) = make_session_and_context().await;
let base_instructions = BaseInstructions {
text: "base".to_string(),
};
let turn_cwd = tempfile::tempdir().expect("temp dir").abs();
#[allow(deprecated)]
{
turn.cwd = turn_cwd.clone();
}
let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config");
assert_eq!(config.cwd, turn_cwd);
assert_eq!(config.workspace_roots, vec![turn_cwd]);
}
#[tokio::test]
async fn build_agent_spawn_config_preserves_base_user_instructions() {
let (_session, mut turn) = make_session_and_context().await;
let mut base_config = (*turn.config).clone();
base_config.user_instructions = Some("base-user".to_string());
turn.user_instructions = Some("resolved-user".to_string());
turn.config = Arc::new(base_config.clone());
let base_instructions = BaseInstructions {
text: "base".to_string(),
};
let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config");
assert_eq!(config.user_instructions, base_config.user_instructions);
}
#[tokio::test]
async fn build_agent_resume_config_clears_base_instructions() {
let (_session, mut turn) = make_session_and_context().await;
+1 -1
View File
@@ -640,7 +640,7 @@ impl TestCodexBuilder {
} else {
load_default_config_for_test(home).await
};
config.cwd = cwd_override;
config.set_cwd_retargeting_implicit_workspace_root(cwd_override);
config.model_provider = model_provider;
if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") {
config.codex_self_exe = Some(path);
+103
View File
@@ -2175,6 +2175,109 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file()
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn same_cwd_apply_patch_with_auto_review_skips_guardian_review() -> Result<()> {
let server = start_mock_server().await;
let approval_policy = AskForApproval::OnRequest;
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let permission_profile = restrictive_workspace_write_profile();
let permission_profile_for_config = permission_profile.clone();
let mut builder = test_codex()
.with_model("gpt-5.4")
.with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
config
.permissions
.set_permission_profile(permission_profile_for_config)
.expect("set permission profile");
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
});
let test = builder.build(&server).await?;
assert_eq!(test.config.workspace_roots, vec![test.config.cwd.clone()]);
let path = test
.config
.cwd
.join("apply_patch_auto_review_inside_workspace.txt")
.into_path_buf();
let patch_path = path.display().to_string();
let _ = fs::remove_file(&path);
let patch = build_add_file_patch(&patch_path, "same-cwd-auto-review");
let call_id = "apply_patch_auto_review_inside_workspace";
let _ = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_apply_patch_custom_tool_call(call_id, &patch),
ev_completed("resp-1"),
]),
)
.await;
let results_mock = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
)
.await;
let session_model = test.session_configured.model.clone();
test.codex
.submit(Op::UserTurn {
environments: None,
items: vec![UserInput::Text {
text: "apply same-cwd patch without guardian review".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.config.cwd.to_path_buf(),
approval_policy,
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
sandbox_policy,
permission_profile: Some(permission_profile),
model: session_model,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
.await?;
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::GuardianAssessment(_) | EventMsg::TurnComplete(_)
)
})
.await;
match event {
EventMsg::TurnComplete(_) => {}
EventMsg::GuardianAssessment(event) => {
panic!("unexpected guardian review for same-cwd apply_patch: {event:?}")
}
other => panic!("unexpected event: {other:?}"),
}
let output_request = results_mock.single_request();
let output_item = output_request.custom_tool_call_output(call_id);
let result = parse_result(&output_item);
assert_eq!(result.exit_code, Some(0));
assert!(fs::read_to_string(&path)?.contains("same-cwd-auto-review"));
let _ = fs::remove_file(path);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> {
@@ -389,6 +389,13 @@ async fn permissions_message_omitted_when_disabled() -> Result<()> {
let mut builder = test_codex().with_config(move |config| {
config.include_permissions_instructions = false;
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let rules_dir = config.codex_home.join("rules");
fs::create_dir_all(&rules_dir).expect("create rules dir");
fs::write(
rules_dir.join("default.rules"),
r#"prefix_rule(pattern=["git", "pull"], decision="allow")"#,
)
.expect("write policy");
});
let test = builder.build(&server).await?;
@@ -437,6 +444,12 @@ async fn permissions_message_omitted_when_disabled() -> Result<()> {
permissions_texts(&req2.single_request()),
Vec::<String>::new()
);
let environment_contexts = environment_context_texts(&req1.single_request());
assert_eq!(environment_contexts.len(), 1);
assert!(
!environment_contexts[0].contains("<approved_command_prefixes>"),
"did not expect approved prefixes when include_permissions_instructions = false: {environment_contexts:?}"
);
Ok(())
}