Skip to content

Conversation

aibrahim-oai
Copy link
Collaborator

Reverts #4286

@aibrahim-oai aibrahim-oai marked this pull request as ready for review October 2, 2025 21:01
@aibrahim-oai aibrahim-oai enabled auto-merge (squash) October 2, 2025 21:01
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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

Comment on lines +2974 to +2981
match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {
// Persist this command as pre‑approved for the
// remainder of the session so future
// executions skip the sandbox directly.
// TODO(ragona): Isn't this a bug? It always saves the command in an | fork?
sess.add_approved_command(params.command.clone()).await;
// Inform UI we are retrying without sandbox.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop treating one-off approvals as session-wide

When a sandboxed exec fails and the user is asked to retry without the sandbox, the code persists the command in the session’s approved set for both ReviewDecision::Approved and ApprovedForSession by calling sess.add_approved_command(...). A user who chooses the single-run "Approved" option will still have the command auto-approved and executed outside the sandbox on every future invocation, which defeats the distinction between one-off approvals and session approvals. The normal approval path only persists the command for ApprovedForSession, so this branch should mirror that behavior.

Useful? React with 👍 / 👎.

@aibrahim-oai aibrahim-oai merged commit ed5d656 into main Oct 2, 2025
19 checks passed
@aibrahim-oai aibrahim-oai deleted the revert-4286-jif/sandbox-1 branch October 2, 2025 21:09
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2025
@tibo-openai tibo-openai changed the title Revert "chore: sanbox extraction" Revert "chore: sandbox extraction" Oct 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants