[codex] Default values from requirements if unset#10531
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 906a731df6
ℹ️ 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".
codex-rs/core/src/config/mod.rs
Outdated
| forced_auto_mode_downgraded_on_windows = true; | ||
| } | ||
| if !sandbox_mode_was_explicit | ||
| && let Some(constraint) = sandbox_policy_constraint | ||
| && let Err(err) = constraint.can_set(&sandbox_policy) |
There was a problem hiding this comment.
Preserve Windows downgrade when applying requirements fallback
On Windows, derive_sandbox_policy forces WorkspaceWrite to ReadOnly when the native sandbox is disabled, but the new requirements fallback runs after this and can overwrite the downgraded policy with the constraint’s required default. If cloud requirements only allow WorkspaceWrite (and no explicit sandbox mode was set), constraint.get() will re‑enable WorkspaceWrite, which is explicitly unsupported in this Windows configuration and leaves forced_auto_mode_downgraded_on_windows inconsistent with the returned policy. Consider re‑checking the Windows downgrade after applying the fallback, or refusing to replace a downgraded policy with an unsupported one.
Useful? React with 👍 / 👎.
jif-oai
left a comment
There was a problem hiding this comment.
Codex is right, rest looks good
If we don't set any explicit values for sandbox or approval policy, let's try to use a requirements-satisfying value.