mirror of
https://github.com/openai/codex.git
synced 2026-05-03 19:06:58 +00:00
## Summary
- bundle contextual prompt injection into at most one developer message
plus one contextual user message in both:
- per-turn settings updates
- initial context insertion
- preserve `<model_switch>` across compaction by rebuilding it through
canonical initial-context injection, instead of relying on
strip/reattach hacks
- centralize contextual user fragment detection in one shared definition
table and reuse it for parsing/compaction logic
- keep `AGENTS.md` in its natural serialized format:
- `# AGENTS.md instructions for {dirname}`
- `<INSTRUCTIONS>...</INSTRUCTIONS>`
- simplify related tests/helpers and accept the expected snapshot/layout
updates from bundled multi-part messages
## Why
The goal is to converge toward a simpler, more intentional prompt shape
where contextual updates are consistently represented as one developer
envelope plus one contextual user envelope, while keeping parsing and
compaction behavior aligned with that representation.
## Notable details
- the temporary `SettingsUpdateEnvelope` wrapper was removed; these
paths now return `Vec<ResponseItem>` directly
- local/remote compaction no longer rely on model-switch strip/restore
helpers
- contextual user detection is now driven by shared fragment definitions
instead of ad hoc matcher assembly
- AGENTS/user instructions are still the same logical context; only the
synthetic `<user_instructions>` wrapper was replaced by the natural
AGENTS text format
## Testing
- `just fmt`
- `cargo test -p codex-app-server
codex_message_processor::tests::extract_conversation_summary_prefers_plain_user_messages
-- --exact`
- `cargo test -p codex-core
compact::tests::collect_user_messages_filters_session_prefix_entries
--lib -- --exact`
- `cargo test -p codex-core --test all
'suite::compact::snapshot_request_shape_pre_turn_compaction_strips_incoming_model_switch'
-- --exact`
- `cargo test -p codex-core --test all
'suite::compact_remote::snapshot_request_shape_remote_pre_turn_compaction_strips_incoming_model_switch'
-- --exact`
- `cargo test -p codex-core --test all
'suite::client::includes_apps_guidance_as_developer_message_when_enabled'
-- --exact`
- `cargo test -p codex-core --test all
'suite::client::includes_developer_instructions_message_in_request' --
--exact`
- `cargo test -p codex-core --test all
'suite::client::includes_user_instructions_message_in_request' --
--exact`
- `cargo test -p codex-core --test all
'suite::client::resume_includes_initial_messages_and_sends_prior_items'
-- --exact`
- `cargo test -p codex-core --test all
'suite::review::review_input_isolated_from_parent_history' -- --exact`
- `cargo test -p codex-exec --test all
'suite::resume::exec_resume_last_respects_cwd_filter_and_all_flag' --
--exact`
- `cargo test -p core_test_support
context_snapshot::tests::full_text_mode_preserves_unredacted_text --
--exact`
## Notes
- I also ran several targeted `compact`, `compact_remote`,
`prompt_caching`, `model_visible_layout`, and `event_mapping` tests
while iterating on prompt-shape changes.
- I have not claimed a clean full-workspace `cargo test` from this
environment because local sandbox/resource conditions have previously
produced unrelated failures in large workspace runs.
126 lines
3.7 KiB
Rust
126 lines
3.7 KiB
Rust
use serde::Deserialize;
|
|
use serde::Serialize;
|
|
|
|
use codex_protocol::models::ResponseItem;
|
|
|
|
use crate::contextual_user_message::AGENTS_MD_FRAGMENT;
|
|
use crate::contextual_user_message::SKILL_FRAGMENT;
|
|
|
|
pub const USER_INSTRUCTIONS_PREFIX: &str = "# AGENTS.md instructions for ";
|
|
|
|
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
|
#[serde(rename = "user_instructions", rename_all = "snake_case")]
|
|
pub(crate) struct UserInstructions {
|
|
pub directory: String,
|
|
pub text: String,
|
|
}
|
|
|
|
impl UserInstructions {
|
|
pub(crate) fn serialize_to_text(&self) -> String {
|
|
format!(
|
|
"{prefix}{directory}\n\n<INSTRUCTIONS>\n{contents}\n{suffix}",
|
|
prefix = AGENTS_MD_FRAGMENT.start_marker(),
|
|
directory = self.directory,
|
|
contents = self.text,
|
|
suffix = AGENTS_MD_FRAGMENT.end_marker(),
|
|
)
|
|
}
|
|
}
|
|
|
|
impl From<UserInstructions> for ResponseItem {
|
|
fn from(ui: UserInstructions) -> Self {
|
|
AGENTS_MD_FRAGMENT.into_message(ui.serialize_to_text())
|
|
}
|
|
}
|
|
|
|
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
|
#[serde(rename = "skill_instructions", rename_all = "snake_case")]
|
|
pub(crate) struct SkillInstructions {
|
|
pub name: String,
|
|
pub path: String,
|
|
pub contents: String,
|
|
}
|
|
|
|
impl SkillInstructions {}
|
|
|
|
impl From<SkillInstructions> for ResponseItem {
|
|
fn from(si: SkillInstructions) -> Self {
|
|
SKILL_FRAGMENT.into_message(SKILL_FRAGMENT.wrap(format!(
|
|
"<name>{}</name>\n<path>{}</path>\n{}",
|
|
si.name, si.path, si.contents
|
|
)))
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use codex_protocol::models::ContentItem;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
#[test]
|
|
fn test_user_instructions() {
|
|
let user_instructions = UserInstructions {
|
|
directory: "test_directory".to_string(),
|
|
text: "test_text".to_string(),
|
|
};
|
|
let response_item: ResponseItem = user_instructions.into();
|
|
|
|
let ResponseItem::Message { role, content, .. } = response_item else {
|
|
panic!("expected ResponseItem::Message");
|
|
};
|
|
|
|
assert_eq!(role, "user");
|
|
|
|
let [ContentItem::InputText { text }] = content.as_slice() else {
|
|
panic!("expected one InputText content item");
|
|
};
|
|
|
|
assert_eq!(
|
|
text,
|
|
"# AGENTS.md instructions for test_directory\n\n<INSTRUCTIONS>\ntest_text\n</INSTRUCTIONS>",
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn test_is_user_instructions() {
|
|
assert!(AGENTS_MD_FRAGMENT.matches_text(
|
|
"# AGENTS.md instructions for test_directory\n\n<INSTRUCTIONS>\ntest_text\n</INSTRUCTIONS>"
|
|
));
|
|
assert!(!AGENTS_MD_FRAGMENT.matches_text("test_text"));
|
|
}
|
|
|
|
#[test]
|
|
fn test_skill_instructions() {
|
|
let skill_instructions = SkillInstructions {
|
|
name: "demo-skill".to_string(),
|
|
path: "skills/demo/SKILL.md".to_string(),
|
|
contents: "body".to_string(),
|
|
};
|
|
let response_item: ResponseItem = skill_instructions.into();
|
|
|
|
let ResponseItem::Message { role, content, .. } = response_item else {
|
|
panic!("expected ResponseItem::Message");
|
|
};
|
|
|
|
assert_eq!(role, "user");
|
|
|
|
let [ContentItem::InputText { text }] = content.as_slice() else {
|
|
panic!("expected one InputText content item");
|
|
};
|
|
|
|
assert_eq!(
|
|
text,
|
|
"<skill>\n<name>demo-skill</name>\n<path>skills/demo/SKILL.md</path>\nbody\n</skill>",
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn test_is_skill_instructions() {
|
|
assert!(SKILL_FRAGMENT.matches_text(
|
|
"<skill>\n<name>demo-skill</name>\n<path>skills/demo/SKILL.md</path>\nbody\n</skill>"
|
|
));
|
|
assert!(!SKILL_FRAGMENT.matches_text("regular text"));
|
|
}
|
|
}
|