Skip to content

Commit 95aae38

Browse files
committed
permissions: require profiles in TUI thread state
1 parent 0b51e47 commit 95aae38

5 files changed

Lines changed: 128 additions & 35 deletions

File tree

codex-rs/tui/src/app/tests.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2212,7 +2212,7 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> {
22122212
ThreadSessionState {
22132213
approval_policy: AskForApproval::OnRequest,
22142214
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2215-
permission_profile: Some(PermissionProfile::workspace_write()),
2215+
permission_profile: PermissionProfile::workspace_write(),
22162216
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
22172217
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
22182218
},
@@ -2372,7 +2372,7 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()>
23722372
ThreadSessionState {
23732373
approval_policy: AskForApproval::OnRequest,
23742374
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2375-
permission_profile: Some(PermissionProfile::workspace_write()),
2375+
permission_profile: PermissionProfile::workspace_write(),
23762376
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
23772377
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
23782378
},
@@ -2664,7 +2664,7 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio
26642664
ThreadSessionState {
26652665
approval_policy: AskForApproval::OnRequest,
26662666
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2667-
permission_profile: Some(PermissionProfile::workspace_write()),
2667+
permission_profile: PermissionProfile::workspace_write(),
26682668
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
26692669
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
26702670
},
@@ -2718,7 +2718,7 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
27182718
let primary_session = ThreadSessionState {
27192719
approval_policy: AskForApproval::OnRequest,
27202720
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2721-
permission_profile: Some(PermissionProfile::workspace_write()),
2721+
permission_profile: PermissionProfile::workspace_write(),
27222722
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
27232723
};
27242724

@@ -2831,7 +2831,7 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_
28312831
let primary_session = ThreadSessionState {
28322832
approval_policy: AskForApproval::OnRequest,
28332833
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2834-
permission_profile: Some(PermissionProfile::workspace_write()),
2834+
permission_profile: PermissionProfile::workspace_write(),
28352835
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
28362836
};
28372837

@@ -2900,7 +2900,7 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
29002900
let primary_session = ThreadSessionState {
29012901
approval_policy: AskForApproval::OnRequest,
29022902
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2903-
permission_profile: Some(PermissionProfile::workspace_write()),
2903+
permission_profile: PermissionProfile::workspace_write(),
29042904
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
29052905
};
29062906
app.primary_session_configured = Some(primary_session);
@@ -2931,10 +2931,17 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
29312931

29322932
assert_eq!(session.thread_id, read_thread_id);
29332933
assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/read").as_path());
2934+
let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
2935+
&app.config
2936+
.permissions
2937+
.legacy_sandbox_policy(thread.cwd.as_path()),
2938+
thread.cwd.as_path(),
2939+
);
29342940
assert_eq!(
2935-
session.permission_profile, None,
2936-
"thread/read does not return an authoritative permission profile; reusing the primary \
2937-
session profile would reinterpret cwd-bound entries against the read thread cwd"
2941+
session.permission_profile, expected_permission_profile,
2942+
"thread/read does not return authoritative server permissions; the fallback profile must \
2943+
be rebuilt from local legacy settings against the read thread cwd rather than reusing \
2944+
the primary session profile"
29382945
);
29392946
}
29402947

@@ -3800,7 +3807,7 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState
38003807
approval_policy: AskForApproval::Never,
38013808
approvals_reviewer: ApprovalsReviewer::User,
38023809
sandbox_policy: SandboxPolicy::new_read_only_policy(),
3803-
permission_profile: Some(PermissionProfile::read_only()),
3810+
permission_profile: PermissionProfile::read_only(),
38043811
cwd: cwd.abs(),
38053812
instruction_source_paths: Vec::new(),
38063813
reasoning_effort: None,

codex-rs/tui/src/app/thread_events.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ mod tests {
351351
approval_policy: AskForApproval::Never,
352352
approvals_reviewer: ApprovalsReviewer::User,
353353
sandbox_policy: SandboxPolicy::new_read_only_policy(),
354-
permission_profile: Some(PermissionProfile::read_only()),
354+
permission_profile: PermissionProfile::read_only(),
355355
cwd: cwd.abs(),
356356
instruction_source_paths: Vec::new(),
357357
reasoning_effort: None,

codex-rs/tui/src/app/thread_session_state.rs

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ use crate::app_server_session::ThreadSessionState;
33
use crate::read_session_model;
44
use codex_app_server_protocol::Thread;
55
use codex_protocol::ThreadId;
6+
use codex_protocol::models::PermissionProfile;
7+
use codex_protocol::protocol::SandboxPolicy;
68

79
impl App {
810
pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) {
@@ -16,12 +18,11 @@ impl App {
1618
.config
1719
.permissions
1820
.legacy_sandbox_policy(self.config.cwd.as_path());
19-
let permission_profile = Some(
20-
self.chat_widget
21-
.config_ref()
22-
.permissions
23-
.permission_profile(),
24-
);
21+
let permission_profile = self
22+
.chat_widget
23+
.config_ref()
24+
.permissions
25+
.permission_profile();
2526
let update_session = |session: &mut ThreadSessionState| {
2627
session.approval_policy = approval_policy;
2728
session.approvals_reviewer = approvals_reviewer;
@@ -48,10 +49,7 @@ impl App {
4849
thread_id: ThreadId,
4950
thread: &Thread,
5051
) -> ThreadSessionState {
51-
let sandbox_policy = self
52-
.config
53-
.permissions
54-
.legacy_sandbox_policy(self.config.cwd.as_path());
52+
let sandbox_policy = self.active_legacy_sandbox_policy_for_cwd(thread.cwd.as_path());
5553
let mut session = self
5654
.primary_session_configured
5755
.clone()
@@ -66,7 +64,8 @@ impl App {
6664
approval_policy: self.config.permissions.approval_policy.value(),
6765
approvals_reviewer: self.config.approvals_reviewer,
6866
sandbox_policy,
69-
permission_profile: None,
67+
permission_profile: self
68+
.active_legacy_permission_profile_for_cwd(thread.cwd.as_path()),
7069
cwd: thread.cwd.clone(),
7170
instruction_source_paths: Vec::new(),
7271
reasoning_effort: self.chat_widget.current_reasoning_effort(),
@@ -79,7 +78,9 @@ impl App {
7978
session.thread_name = thread.name.clone();
8079
session.model_provider_id = thread.model_provider.clone();
8180
session.cwd = thread.cwd.clone();
82-
session.permission_profile = None;
81+
session.sandbox_policy = self.active_legacy_sandbox_policy_for_cwd(thread.cwd.as_path());
82+
session.permission_profile =
83+
self.active_legacy_permission_profile_for_cwd(thread.cwd.as_path());
8384
session.instruction_source_paths = Vec::new();
8485
session.rollout_path = thread.path.clone();
8586
if let Some(model) =
@@ -93,6 +94,18 @@ impl App {
9394
session.history_entry_count = 0;
9495
session
9596
}
97+
98+
fn active_legacy_permission_profile_for_cwd(&self, cwd: &std::path::Path) -> PermissionProfile {
99+
let sandbox_policy = self.active_legacy_sandbox_policy_for_cwd(cwd);
100+
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, cwd)
101+
}
102+
103+
fn active_legacy_sandbox_policy_for_cwd(&self, cwd: &std::path::Path) -> SandboxPolicy {
104+
self.chat_widget
105+
.config_ref()
106+
.permissions
107+
.legacy_sandbox_policy(cwd)
108+
}
96109
}
97110

98111
#[cfg(test)]
@@ -128,7 +141,7 @@ mod tests {
128141
approval_policy: AskForApproval::Never,
129142
approvals_reviewer: ApprovalsReviewer::User,
130143
sandbox_policy: SandboxPolicy::new_read_only_policy(),
131-
permission_profile: None,
144+
permission_profile: PermissionProfile::read_only(),
132145
cwd: cwd.abs(),
133146
instruction_source_paths: Vec::new(),
134147
reasoning_effort: None,
@@ -202,7 +215,7 @@ mod tests {
202215
approval_policy: AskForApproval::OnRequest,
203216
approvals_reviewer: ApprovalsReviewer::AutoReview,
204217
sandbox_policy: expected_sandbox_policy,
205-
permission_profile: Some(expected_permission_profile),
218+
permission_profile: expected_permission_profile,
206219
..main_session
207220
};
208221
assert_eq!(
@@ -256,7 +269,7 @@ mod tests {
256269
NetworkSandboxPolicy::Restricted,
257270
);
258271
let session = ThreadSessionState {
259-
permission_profile: Some(profile.clone()),
272+
permission_profile: profile.clone(),
260273
..test_thread_session(thread_id, test_path_buf("/tmp/main"))
261274
};
262275

@@ -276,7 +289,7 @@ mod tests {
276289

277290
let expected_session = ThreadSessionState {
278291
approval_policy: AskForApproval::OnRequest,
279-
permission_profile: Some(profile),
292+
permission_profile: profile,
280293
..session
281294
};
282295
assert_eq!(
@@ -295,4 +308,61 @@ mod tests {
295308
.clone();
296309
assert_eq!(store_session, Some(expected_session));
297310
}
311+
312+
#[tokio::test]
313+
async fn thread_read_fallback_uses_active_permission_settings() {
314+
let mut app = make_test_app().await;
315+
let primary_thread_id =
316+
ThreadId::from_string("00000000-0000-0000-0000-000000000404").expect("valid thread");
317+
let read_thread_id =
318+
ThreadId::from_string("00000000-0000-0000-0000-000000000405").expect("valid thread");
319+
let primary_session = ThreadSessionState {
320+
permission_profile: PermissionProfile::legacy_workspace_write_template(),
321+
..test_thread_session(primary_thread_id, test_path_buf("/tmp/primary"))
322+
};
323+
let read_thread = Thread {
324+
id: read_thread_id.to_string(),
325+
forked_from_id: None,
326+
preview: "read thread".to_string(),
327+
ephemeral: false,
328+
model_provider: "read-provider".to_string(),
329+
created_at: 1,
330+
updated_at: 2,
331+
status: codex_app_server_protocol::ThreadStatus::Idle,
332+
path: None,
333+
cwd: test_path_buf("/tmp/read").abs(),
334+
cli_version: "0.0.0".to_string(),
335+
source: codex_app_server_protocol::SessionSource::Unknown,
336+
agent_nickname: None,
337+
agent_role: None,
338+
git_info: None,
339+
name: Some("read thread".to_string()),
340+
turns: Vec::new(),
341+
};
342+
343+
app.primary_session_configured = Some(primary_session.clone());
344+
app.chat_widget.handle_thread_session(primary_session);
345+
346+
let session = app
347+
.session_state_for_thread_read(read_thread_id, &read_thread)
348+
.await;
349+
350+
let expected_sandbox_policy = app
351+
.chat_widget
352+
.config_ref()
353+
.permissions
354+
.legacy_sandbox_policy(read_thread.cwd.as_path());
355+
let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
356+
&expected_sandbox_policy,
357+
read_thread.cwd.as_path(),
358+
);
359+
assert_eq!(session.sandbox_policy, expected_sandbox_policy);
360+
assert_eq!(session.permission_profile, expected_permission_profile);
361+
assert_ne!(
362+
session.permission_profile,
363+
app.config.permissions.permission_profile(),
364+
"thread/read fallback must use the active widget permissions rather than stale app \
365+
config defaults"
366+
);
367+
}
298368
}

codex-rs/tui/src/app_server_session.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,13 @@ pub(crate) struct ThreadSessionState {
156156
pub(crate) service_tier: Option<codex_protocol::config_types::ServiceTier>,
157157
pub(crate) approval_policy: AskForApproval,
158158
pub(crate) approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer,
159-
/// Legacy sandbox projection kept for compatibility. Use this only when
160-
/// `permission_profile` is `None`.
159+
/// Legacy sandbox projection kept only for compatibility fields that have
160+
/// not migrated to `PermissionProfile` yet.
161161
pub(crate) sandbox_policy: SandboxPolicy,
162-
/// Canonical active permissions when available. Consumers should prefer
163-
/// this over `sandbox_policy`; `None` means the session only has a legacy
164-
/// sandbox projection.
165-
pub(crate) permission_profile: Option<PermissionProfile>,
162+
/// Canonical active permissions for this session. Legacy app-server
163+
/// responses are converted to a profile at ingestion time using the
164+
/// response cwd so cached sessions do not reinterpret cwd-bound grants.
165+
pub(crate) permission_profile: PermissionProfile,
166166
pub(crate) cwd: AbsolutePathBuf,
167167
pub(crate) instruction_source_paths: Vec<AbsolutePathBuf>,
168168
pub(crate) reasoning_effort: Option<codex_protocol::openai_models::ReasoningEffort>,
@@ -1407,6 +1407,8 @@ async fn thread_session_state_from_thread_response(
14071407
.map_err(|err| format!("forked_from_id is invalid: {err}"))?;
14081408
let (history_log_id, history_entry_count) = message_history_metadata(config).await;
14091409
let history_entry_count = u64::try_from(history_entry_count).unwrap_or(u64::MAX);
1410+
let permission_profile =
1411+
permission_profile_from_response_permissions(&sandbox_policy, permission_profile, &cwd);
14101412

14111413
Ok(ThreadSessionState {
14121414
thread_id,
@@ -1430,6 +1432,16 @@ async fn thread_session_state_from_thread_response(
14301432
})
14311433
}
14321434

1435+
fn permission_profile_from_response_permissions(
1436+
sandbox_policy: &SandboxPolicy,
1437+
permission_profile: Option<PermissionProfile>,
1438+
cwd: &AbsolutePathBuf,
1439+
) -> PermissionProfile {
1440+
permission_profile.unwrap_or_else(|| {
1441+
PermissionProfile::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd.as_path())
1442+
})
1443+
}
1444+
14331445
pub(crate) fn app_server_rate_limit_snapshots_to_core(
14341446
response: GetAccountRateLimitsResponse,
14351447
) -> Vec<RateLimitSnapshot> {
@@ -1770,7 +1782,11 @@ mod tests {
17701782
);
17711783
assert_eq!(
17721784
started.session.permission_profile,
1773-
response.permission_profile.clone().map(Into::into)
1785+
response
1786+
.permission_profile
1787+
.clone()
1788+
.map(Into::into)
1789+
.expect("response includes profile")
17741790
);
17751791
assert_eq!(started.turns.len(), 1);
17761792
assert_eq!(started.turns[0], response.thread.turns[0]);

codex-rs/tui/src/chatwidget.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1619,7 +1619,7 @@ fn thread_session_state_to_legacy_event(
16191619
approval_policy: session.approval_policy,
16201620
approvals_reviewer: session.approvals_reviewer,
16211621
sandbox_policy: session.sandbox_policy,
1622-
permission_profile: session.permission_profile,
1622+
permission_profile: Some(session.permission_profile),
16231623
cwd: session.cwd,
16241624
reasoning_effort: session.reasoning_effort,
16251625
history_log_id: session.history_log_id,

0 commit comments

Comments
 (0)