mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
app-server: stop returning thread permission profiles (#22792)
## Why The app-server thread lifecycle API should no longer expose the full `PermissionProfile` value. After the permissions-profile migration, clients should round-trip only the active profile identity through `activePermissionProfile` and `permissions` when that identity is known. The full profile is server-side config. Treating a response-derived legacy sandbox projection as a new local profile can lose named-profile restrictions and accidentally widen permissions on the next turn. The legacy `sandbox` response field remains only as the compatibility/display fallback. ## What Changed - Removed `permissionProfile` from `ThreadStartResponse`, `ThreadResumeResponse`, and `ThreadForkResponse`. - Stopped populating that field in app-server thread start/resume/fork responses. - Updated embedded exec/TUI response mapping to derive display permission state from local config or the legacy sandbox fallback instead of a response profile value. - Added a TUI turn override shape that distinguishes preserving server permissions, selecting an active profile id, and sending a legacy sandbox for an explicit local override. - Preserved remote app-server permissions across turns by sending `permissions` only when an `activePermissionProfile` id is known, and otherwise sending no sandbox override unless the user selected a local override. - Kept embedded `thread/resume` hydration server-authored when `activePermissionProfile` is absent, which matches the live-thread attach path where the server ignores requested overrides. - Updated the app-server README to remove the obsolete lifecycle response `permissionProfile` reference. The remaining `permissionProfile` README references are request-side permission overrides. - Regenerated app-server JSON schema and TypeScript fixtures. - Kept the generated typed response enum exempt from `large_enum_variant`, matching the existing payload enum exemption after the lifecycle response variants shrank. ## How To Review Start with `codex-rs/app-server-protocol/src/protocol/v2/thread.rs` to confirm the response shape, then check the response construction in `codex-rs/app-server/src/request_processors`. The generated schema and TypeScript fixture changes are mechanical follow-through from the protocol removal. The TUI behavior is the delicate part: review `codex-rs/tui/src/app_server_session.rs` for response hydration and turn-start override projection, then `codex-rs/tui/src/app/thread_routing.rs` for the decision about whether the next turn should preserve the server snapshot, send an active profile id, or send a legacy sandbox for an explicit local override. ## Verification - `just write-app-server-schema` - `cargo test -p codex-app-server-protocol thread_lifecycle_responses_default_missing_optional_fields` - `cargo test -p codex-exec session_configured_from_thread_response_uses_permission_profile_from_config` - `cargo test -p codex-tui --lib thread_response` - `cargo test -p codex-tui turn_permissions_` - `cargo test -p codex-tui resume_response_restores_turns_from_thread_items` - `cargo test -p codex-analytics track_response_only_enqueues_analytics_relevant_responses` - `just fix -p codex-analytics` - `just fix -p codex-app-server-protocol` - `just fix -p codex-tui` - `just argument-comment-lint` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22792). * #22795 * __->__ #22792
This commit is contained in:
@@ -588,14 +588,11 @@ impl App {
|
||||
let config = self.chat_widget.config_ref();
|
||||
let approvals_reviewer =
|
||||
approvals_reviewer.unwrap_or(config.approvals_reviewer);
|
||||
let active_permission_profile =
|
||||
if config.permissions.effective_permission_profile()
|
||||
== permission_profile.clone()
|
||||
{
|
||||
config.permissions.active_permission_profile()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let permissions_override = Self::turn_permissions_override_from_config(
|
||||
config,
|
||||
permission_profile,
|
||||
self.runtime_permission_profile_override.as_ref(),
|
||||
);
|
||||
app_server
|
||||
.turn_start(
|
||||
thread_id,
|
||||
@@ -603,8 +600,7 @@ impl App {
|
||||
cwd.clone(),
|
||||
*approval_policy,
|
||||
approvals_reviewer,
|
||||
permission_profile.clone(),
|
||||
active_permission_profile,
|
||||
permissions_override,
|
||||
config.permissions.user_visible_workspace_roots(),
|
||||
model.to_string(),
|
||||
*effort,
|
||||
@@ -700,6 +696,36 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn turn_permissions_override_from_config(
|
||||
config: &Config,
|
||||
permission_profile: &PermissionProfile,
|
||||
runtime_permission_profile_override: Option<&PermissionProfile>,
|
||||
) -> TurnPermissionsOverride {
|
||||
let effective_permission_profile = config.permissions.effective_permission_profile();
|
||||
if &effective_permission_profile == permission_profile
|
||||
&& let Some(active_permission_profile) = config.permissions.active_permission_profile()
|
||||
{
|
||||
return TurnPermissionsOverride::ActiveProfile(active_permission_profile);
|
||||
}
|
||||
|
||||
let runtime_permission_profile_override =
|
||||
runtime_permission_profile_override.map(|profile| {
|
||||
profile
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(
|
||||
&config.effective_workspace_roots(),
|
||||
)
|
||||
});
|
||||
if runtime_permission_profile_override
|
||||
.as_ref()
|
||||
.is_some_and(|profile| profile == permission_profile)
|
||||
{
|
||||
return TurnPermissionsOverride::LegacySandbox(permission_profile.clone());
|
||||
}
|
||||
|
||||
TurnPermissionsOverride::Preserve
|
||||
}
|
||||
|
||||
pub(super) fn handle_skills_list_result(
|
||||
&mut self,
|
||||
result: Result<SkillsListResponse>,
|
||||
@@ -1457,3 +1483,79 @@ impl App {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
|
||||
async fn config_with_workspace_profile() -> Config {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
ConfigBuilder::default()
|
||||
.codex_home(temp_dir.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
default_permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
|
||||
..ConfigOverrides::default()
|
||||
})
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_permissions_use_active_profile_when_available() {
|
||||
let config = config_with_workspace_profile().await;
|
||||
let permission_profile = config.permissions.effective_permission_profile();
|
||||
|
||||
assert_eq!(
|
||||
App::turn_permissions_override_from_config(
|
||||
&config,
|
||||
&permission_profile,
|
||||
/*runtime_permission_profile_override*/ None,
|
||||
),
|
||||
TurnPermissionsOverride::ActiveProfile(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_permissions_preserve_server_snapshot_without_local_override() {
|
||||
let mut config = config_with_workspace_profile().await;
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::read_only())
|
||||
.expect("read-only profile should be allowed");
|
||||
let permission_profile = config.permissions.effective_permission_profile();
|
||||
|
||||
assert_eq!(
|
||||
App::turn_permissions_override_from_config(
|
||||
&config,
|
||||
&permission_profile,
|
||||
/*runtime_permission_profile_override*/ None,
|
||||
),
|
||||
TurnPermissionsOverride::Preserve
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_permissions_send_legacy_sandbox_for_local_override() {
|
||||
let mut config = config_with_workspace_profile().await;
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(permission_profile.clone())
|
||||
.expect("workspace profile should be allowed");
|
||||
let effective_permission_profile = config.permissions.effective_permission_profile();
|
||||
|
||||
assert_eq!(
|
||||
App::turn_permissions_override_from_config(
|
||||
&config,
|
||||
&effective_permission_profile,
|
||||
Some(&permission_profile),
|
||||
),
|
||||
TurnPermissionsOverride::LegacySandbox(effective_permission_profile)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user