core: bundle settings diff updates into one dev/user envelope#12417
core: bundle settings diff updates into one dev/user envelope#12417charley-oai wants to merge 4 commits intomainfrom
Conversation
charley-oai
force-pushed
the
cc/context-update-envelope-single-message
branch
from
25df28f to
acdc8ec
Compare
|
@codex review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acdc8ec47d
ℹ️ 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".
| let content = update_sections | ||
| .into_iter() | ||
| .map(|section| ContentItem::InputText { | ||
| text: section.into_text(), | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
Bundling all developer diffs into one message creates multi-item content. compact::extract_trailing_model_switch_update_for_compaction_request only treats a developer item as a model-switch update when content is exactly one InputText starting with <model_switch>. If model switch is bundled with permissions/personality, compaction won’t strip+reattach it, so switch guidance is dropped when history is replaced.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: acdc8ec47d
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| if let Some(developer_message) = self.developer_message { | ||
| items.push(developer_message); | ||
| } | ||
| if let Some(contextual_user_message) = self.contextual_user_message { | ||
| items.push(contextual_user_message); |
There was a problem hiding this comment.
SettingsUpdateEnvelope::into_items now emits the merged developer update before the contextual user environment diff. Because environment context contains unescaped cwd/domain strings (EnvironmentContext::serialize_to_xml), attacker-influenced paths can inject instruction-like text in a later user message immediately before the real user prompt. This weakens instruction precedence for permissions/collaboration guidance that used to follow contextual data. Keep contextual user updates before policy developer updates (or sanitize/escape environment fields) to avoid prompt-injection-by-ordering regressions.
Useful? React with 👍 / 👎.
1 participant
Summary
ResponseItemwhile keeping sections as separateInputTextentriesWhy
This is a minimal step toward the one-developer/one-contextual-user message shape for turn context diffs, while preserving compaction trimming behavior for contextual user messages.
Testing
just fmtcargo test -p codex-core skips_user_instructions_and_env --libcargo test -p codex-core build_settings_update_items_emits_environment_item_for_network_changescargo test -p codex-core process_compacted_history_drops_non_user_content_messagescargo test -p codex-core refresh_mcp_servers_is_deferred_until_next_turn --libNotes
codex.rsto keep the diff smaller and easier to review; behavior is unchanged.