Skip to content

Commit dbcbf3b

Browse files
bolinfestbolinfest
committed
permissions: require profiles in TUI thread state
1 parent d4eb184 commit dbcbf3b

5 files changed

Lines changed: 64 additions & 32 deletions

File tree

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,7 +2208,7 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> {
22082208
ThreadSessionState {
22092209
approval_policy: AskForApproval::OnRequest,
22102210
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2211-
permission_profile: Some(PermissionProfile::workspace_write()),
2211+
permission_profile: PermissionProfile::workspace_write(),
22122212
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
22132213
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
22142214
},
@@ -2368,7 +2368,7 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()>
23682368
ThreadSessionState {
23692369
approval_policy: AskForApproval::OnRequest,
23702370
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2371-
permission_profile: Some(PermissionProfile::workspace_write()),
2371+
permission_profile: PermissionProfile::workspace_write(),
23722372
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
23732373
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
23742374
},
@@ -2591,7 +2591,7 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio
25912591
ThreadSessionState {
25922592
approval_policy: AskForApproval::OnRequest,
25932593
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2594-
permission_profile: Some(PermissionProfile::workspace_write()),
2594+
permission_profile: PermissionProfile::workspace_write(),
25952595
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
25962596
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
25972597
},
@@ -2645,7 +2645,7 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
26452645
let primary_session = ThreadSessionState {
26462646
approval_policy: AskForApproval::OnRequest,
26472647
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2648-
permission_profile: Some(PermissionProfile::workspace_write()),
2648+
permission_profile: PermissionProfile::workspace_write(),
26492649
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
26502650
};
26512651

@@ -2758,7 +2758,7 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_
27582758
let primary_session = ThreadSessionState {
27592759
approval_policy: AskForApproval::OnRequest,
27602760
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2761-
permission_profile: Some(PermissionProfile::workspace_write()),
2761+
permission_profile: PermissionProfile::workspace_write(),
27622762
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
27632763
};
27642764

@@ -2827,7 +2827,7 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
28272827
let primary_session = ThreadSessionState {
28282828
approval_policy: AskForApproval::OnRequest,
28292829
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
2830-
permission_profile: Some(PermissionProfile::workspace_write()),
2830+
permission_profile: PermissionProfile::workspace_write(),
28312831
..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
28322832
};
28332833
app.primary_session_configured = Some(primary_session);
@@ -2858,10 +2858,17 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
28582858

28592859
assert_eq!(session.thread_id, read_thread_id);
28602860
assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/read").as_path());
2861+
let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
2862+
&app.config
2863+
.permissions
2864+
.legacy_sandbox_policy(thread.cwd.as_path()),
2865+
thread.cwd.as_path(),
2866+
);
28612867
assert_eq!(
2862-
session.permission_profile, None,
2863-
"thread/read does not return an authoritative permission profile; reusing the primary \
2864-
session profile would reinterpret cwd-bound entries against the read thread cwd"
2868+
session.permission_profile, expected_permission_profile,
2869+
"thread/read does not return authoritative server permissions; the fallback profile must \
2870+
be rebuilt from local legacy settings against the read thread cwd rather than reusing \
2871+
the primary session profile"
28652872
);
28662873
}
28672874

@@ -3727,7 +3734,7 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState
37273734
approval_policy: AskForApproval::Never,
37283735
approvals_reviewer: ApprovalsReviewer::User,
37293736
sandbox_policy: SandboxPolicy::new_read_only_policy(),
3730-
permission_profile: Some(PermissionProfile::read_only()),
3737+
permission_profile: PermissionProfile::read_only(),
37313738
cwd: cwd.abs(),
37323739
instruction_source_paths: Vec::new(),
37333740
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
@@ -303,7 +303,7 @@ mod tests {
303303
approval_policy: AskForApproval::Never,
304304
approvals_reviewer: ApprovalsReviewer::User,
305305
sandbox_policy: SandboxPolicy::new_read_only_policy(),
306-
permission_profile: Some(PermissionProfile::read_only()),
306+
permission_profile: PermissionProfile::read_only(),
307307
cwd: cwd.abs(),
308308
instruction_source_paths: Vec::new(),
309309
reasoning_effort: None,

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ 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;
67

78
impl App {
89
pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) {
@@ -16,12 +17,11 @@ impl App {
1617
.config
1718
.permissions
1819
.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-
);
20+
let permission_profile = self
21+
.chat_widget
22+
.config_ref()
23+
.permissions
24+
.permission_profile();
2525
let update_session = |session: &mut ThreadSessionState| {
2626
session.approval_policy = approval_policy;
2727
session.approvals_reviewer = approvals_reviewer;
@@ -51,7 +51,7 @@ impl App {
5151
let sandbox_policy = self
5252
.config
5353
.permissions
54-
.legacy_sandbox_policy(self.config.cwd.as_path());
54+
.legacy_sandbox_policy(thread.cwd.as_path());
5555
let mut session = self
5656
.primary_session_configured
5757
.clone()
@@ -66,7 +66,7 @@ impl App {
6666
approval_policy: self.config.permissions.approval_policy.value(),
6767
approvals_reviewer: self.config.approvals_reviewer,
6868
sandbox_policy,
69-
permission_profile: None,
69+
permission_profile: self.legacy_permission_profile_for_cwd(thread.cwd.as_path()),
7070
cwd: thread.cwd.clone(),
7171
instruction_source_paths: Vec::new(),
7272
reasoning_effort: self.chat_widget.current_reasoning_effort(),
@@ -79,7 +79,11 @@ impl App {
7979
session.thread_name = thread.name.clone();
8080
session.model_provider_id = thread.model_provider.clone();
8181
session.cwd = thread.cwd.clone();
82-
session.permission_profile = None;
82+
session.sandbox_policy = self
83+
.config
84+
.permissions
85+
.legacy_sandbox_policy(thread.cwd.as_path());
86+
session.permission_profile = self.legacy_permission_profile_for_cwd(thread.cwd.as_path());
8387
session.instruction_source_paths = Vec::new();
8488
session.rollout_path = thread.path.clone();
8589
if let Some(model) =
@@ -93,6 +97,11 @@ impl App {
9397
session.history_entry_count = 0;
9498
session
9599
}
100+
101+
fn legacy_permission_profile_for_cwd(&self, cwd: &std::path::Path) -> PermissionProfile {
102+
let sandbox_policy = self.config.permissions.legacy_sandbox_policy(cwd);
103+
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, cwd)
104+
}
96105
}
97106

98107
#[cfg(test)]
@@ -128,7 +137,7 @@ mod tests {
128137
approval_policy: AskForApproval::Never,
129138
approvals_reviewer: ApprovalsReviewer::User,
130139
sandbox_policy: SandboxPolicy::new_read_only_policy(),
131-
permission_profile: None,
140+
permission_profile: PermissionProfile::read_only(),
132141
cwd: cwd.abs(),
133142
instruction_source_paths: Vec::new(),
134143
reasoning_effort: None,
@@ -202,7 +211,7 @@ mod tests {
202211
approval_policy: AskForApproval::OnRequest,
203212
approvals_reviewer: ApprovalsReviewer::AutoReview,
204213
sandbox_policy: expected_sandbox_policy,
205-
permission_profile: Some(expected_permission_profile),
214+
permission_profile: expected_permission_profile,
206215
..main_session
207216
};
208217
assert_eq!(
@@ -256,7 +265,7 @@ mod tests {
256265
NetworkSandboxPolicy::Restricted,
257266
);
258267
let session = ThreadSessionState {
259-
permission_profile: Some(profile.clone()),
268+
permission_profile: profile.clone(),
260269
..test_thread_session(thread_id, test_path_buf("/tmp/main"))
261270
};
262271

@@ -276,7 +285,7 @@ mod tests {
276285

277286
let expected_session = ThreadSessionState {
278287
approval_policy: AskForApproval::OnRequest,
279-
permission_profile: Some(profile),
288+
permission_profile: profile,
280289
..session
281290
};
282291
assert_eq!(

‎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
@@ -1618,7 +1618,7 @@ fn thread_session_state_to_legacy_event(
16181618
approval_policy: session.approval_policy,
16191619
approvals_reviewer: session.approvals_reviewer,
16201620
sandbox_policy: session.sandbox_policy,
1621-
permission_profile: session.permission_profile,
1621+
permission_profile: Some(session.permission_profile),
16221622
cwd: session.cwd,
16231623
reasoning_effort: session.reasoning_effort,
16241624
history_log_id: session.history_log_id,

0 commit comments

Comments
 (0)