Skip to content

protocol: require permission profiles for user turns#20441

Open
bolinfest wants to merge 1 commit intopr20440from
pr20441
Open

protocol: require permission profiles for user turns#20441
bolinfest wants to merge 1 commit intopr20440from
pr20441

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 30, 2026

Why

Op::UserTurn is the explicit turn submission path where callers already provide the complete turn context. Keeping permission_profile optional meant core still had to preserve a legacy SandboxPolicy fallback for this path, even though every migrated caller should know the canonical permissions profile at submission time.

This PR makes permission_profile required for UserTurn so core no longer has to derive command permissions from the legacy sandbox field for explicit turns. The legacy sandbox_policy slot is left in place temporarily but is ignored for UserTurn; follow-up PRs can remove it once the remaining mechanical test/helper callsites stop carrying it.

What Changed

  • Changes Op::UserTurn.permission_profile from Option<PermissionProfile> to required PermissionProfile.
  • Updates core turn handling to use the required profile directly instead of calling the legacy fallback helper.
  • Updates TUI command conversion and tests for the non-optional profile shape.
  • Keeps UserInputWithTurnContext and OverrideTurnContext legacy fallback behavior unchanged because those are still compatibility update surfaces.

Verification

  • cargo check -p codex-protocol -p codex-core -p codex-tui --tests

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest requested a review from a team as a code owner April 30, 2026 15:22
@bolinfest bolinfest changed the base branch from main to pr20440 April 30, 2026 15:22
This was referenced Apr 30, 2026
@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

// Current app-server responses include `permissionProfile`. If it is absent,
// keep the TUI on its configured profile rather than rebuilding a lossy
// profile from the legacy `sandbox` compatibility field.
config.permissions.permission_profile()

P2 Badge Restore sandbox fallback when permissionProfile is missing

When thread/start, thread/resume, or thread/fork responses omit permissionProfile, this now always falls back to local config permissions instead of deriving from the response sandbox field (the previous remote-mode behavior). In mixed-version setups (new TUI talking to an older remote app-server that only sends sandbox), the TUI can display and submit turns with incorrect permissions, causing mismatched approval/sandbox UX and incorrect turn overrides.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant