mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
app-server: use profile ids in v2 permission params (#23360)
## Why The v2 app-server permission profile fields are experimental, but the previous migration kept a legacy object payload for profile selection. That made clients aware of server-owned `activePermissionProfile` metadata such as `extends`, and it kept a `legacy_additional_writable_roots` path even though `runtimeWorkspaceRoots` now owns runtime workspace-root selection. This PR makes the client contract match the intended model: clients select a permission profile by id, and the server resolves and reports active profile provenance in response payloads. Follow-up to #22611. ## What Changed - Changed `thread/start`, `thread/resume`, `thread/fork`, and `turn/start` permission profile selection to plain profile id strings. - Changed `command/exec.permissionProfile` to a plain profile id string for the same client/server ownership split. - Removed `PermissionProfileSelectionParams` and the legacy `{ type: "profile", modifications: [...] }` compatibility deserializer. - Updated app-server, TUI, and `codex exec` call sites to send only ids, while keeping `activePermissionProfile` as server response metadata. - Updated app-server docs and schema fixtures for the revised `command/exec.permissionProfile` shape. ## Verification - `cargo test -p codex-app-server-protocol` - `RUST_MIN_STACK=8388608 cargo test -p codex-app-server` - `cargo test -p codex-exec` - `RUST_MIN_STACK=8388608 cargo test -p codex-tui` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23360). * #23368 * __->__ #23360
This commit is contained in:
@@ -117,13 +117,6 @@ impl CommandExecRequestProcessor {
|
||||
"`permissionProfile` cannot be combined with `sandboxPolicy`",
|
||||
));
|
||||
}
|
||||
let permission_profile = if let Some(active_permission_profile) = permission_profile {
|
||||
Some(PermissionProfileSelectionParams::new(
|
||||
active_permission_profile.id,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
if size.is_some() && !tty {
|
||||
return Err(invalid_params("command/exec size requires tty: true"));
|
||||
@@ -199,14 +192,11 @@ impl CommandExecRequestProcessor {
|
||||
network_proxy_permission_profile,
|
||||
managed_network_requirements_enabled,
|
||||
) = if let Some(permission_profile) = permission_profile {
|
||||
let mut overrides = ConfigOverrides {
|
||||
let overrides = ConfigOverrides {
|
||||
cwd: Some(cwd.to_path_buf()),
|
||||
default_permissions: Some(permission_profile),
|
||||
..Default::default()
|
||||
};
|
||||
apply_permission_profile_selection_to_config_overrides(
|
||||
&mut overrides,
|
||||
Some(permission_profile),
|
||||
);
|
||||
let config = self
|
||||
.config_manager
|
||||
.load_for_cwd(
|
||||
|
||||
@@ -1239,17 +1239,18 @@ impl ThreadRequestProcessor {
|
||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||
sandbox: Option<SandboxMode>,
|
||||
permissions: Option<PermissionProfileSelectionParams>,
|
||||
permissions: Option<String>,
|
||||
base_instructions: Option<String>,
|
||||
developer_instructions: Option<String>,
|
||||
personality: Option<Personality>,
|
||||
) -> ConfigOverrides {
|
||||
let mut overrides = ConfigOverrides {
|
||||
ConfigOverrides {
|
||||
model,
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd: cwd.map(PathBuf::from),
|
||||
workspace_roots: runtime_workspace_roots,
|
||||
default_permissions: permissions,
|
||||
approval_policy: approval_policy
|
||||
.map(codex_app_server_protocol::AskForApproval::to_core),
|
||||
approvals_reviewer: approvals_reviewer
|
||||
@@ -1261,9 +1262,7 @@ impl ThreadRequestProcessor {
|
||||
developer_instructions,
|
||||
personality,
|
||||
..Default::default()
|
||||
};
|
||||
apply_permission_profile_selection_to_config_overrides(&mut overrides, permissions);
|
||||
overrides
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_environment_selections(
|
||||
|
||||
@@ -175,29 +175,6 @@ pub(super) fn thread_response_active_permission_profile(
|
||||
active_permission_profile.map(Into::into)
|
||||
}
|
||||
|
||||
pub(super) fn apply_permission_profile_selection_to_config_overrides(
|
||||
overrides: &mut ConfigOverrides,
|
||||
permissions: Option<PermissionProfileSelectionParams>,
|
||||
) {
|
||||
let Some(selection) = permissions else {
|
||||
return;
|
||||
};
|
||||
overrides.default_permissions = Some(selection.id().to_string());
|
||||
if selection.legacy_additional_writable_roots().is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let legacy_roots = selection
|
||||
.legacy_additional_writable_roots()
|
||||
.iter()
|
||||
.map(AbsolutePathBuf::to_path_buf);
|
||||
if let Some(workspace_roots) = overrides.workspace_roots.as_mut() {
|
||||
workspace_roots.extend(legacy_roots);
|
||||
} else {
|
||||
overrides.additional_writable_roots.extend(legacy_roots);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn thread_response_sandbox_policy(
|
||||
permission_profile: &codex_protocol::models::PermissionProfile,
|
||||
cwd: &Path,
|
||||
|
||||
@@ -66,43 +66,3 @@ fn extract_conversation_summary_prefers_plain_user_messages() -> Result<()> {
|
||||
assert_eq!(summary, expected);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_permission_profile_modifications_extend_runtime_roots() -> Result<()> {
|
||||
let root = if cfg!(windows) {
|
||||
AbsolutePathBuf::try_from("C:\\workspace-extra")?
|
||||
} else {
|
||||
AbsolutePathBuf::try_from("/workspace-extra")?
|
||||
};
|
||||
let selection = serde_json::from_value::<PermissionProfileSelectionParams>(json!({
|
||||
"type": "profile",
|
||||
"id": ":workspace",
|
||||
"modifications": [
|
||||
{
|
||||
"type": "additionalWritableRoot",
|
||||
"path": root,
|
||||
}
|
||||
],
|
||||
}))?;
|
||||
|
||||
let mut overrides = ConfigOverrides::default();
|
||||
apply_permission_profile_selection_to_config_overrides(&mut overrides, Some(selection.clone()));
|
||||
assert_eq!(
|
||||
overrides.default_permissions,
|
||||
Some(":workspace".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
overrides.additional_writable_roots,
|
||||
vec![root.to_path_buf()]
|
||||
);
|
||||
|
||||
let mut overrides = ConfigOverrides {
|
||||
workspace_roots: Some(Vec::new()),
|
||||
..ConfigOverrides::default()
|
||||
};
|
||||
apply_permission_profile_selection_to_config_overrides(&mut overrides, Some(selection));
|
||||
assert_eq!(overrides.additional_writable_roots, Vec::<PathBuf>::new());
|
||||
assert_eq!(overrides.workspace_roots, Some(vec![root.to_path_buf()]));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -425,7 +425,7 @@ impl TurnRequestProcessor {
|
||||
"turn/start permission selection missing thread snapshot",
|
||||
));
|
||||
};
|
||||
let mut overrides = ConfigOverrides {
|
||||
let overrides = ConfigOverrides {
|
||||
cwd: cwd.clone(),
|
||||
workspace_roots: Some(runtime_workspace_roots_request.clone().unwrap_or_else(
|
||||
|| {
|
||||
@@ -436,14 +436,11 @@ impl TurnRequestProcessor {
|
||||
.collect()
|
||||
},
|
||||
)),
|
||||
default_permissions: Some(permissions),
|
||||
codex_linux_sandbox_exe: self.arg0_paths.codex_linux_sandbox_exe.clone(),
|
||||
main_execve_wrapper_exe: self.arg0_paths.main_execve_wrapper_exe.clone(),
|
||||
..Default::default()
|
||||
};
|
||||
apply_permission_profile_selection_to_config_overrides(
|
||||
&mut overrides,
|
||||
Some(permissions),
|
||||
);
|
||||
let config = self
|
||||
.config_manager
|
||||
.load_for_cwd(
|
||||
|
||||
Reference in New Issue
Block a user