Skip to content

Commit b5d8350

Browse files
charley-oaicodex
andcommitted
refactor(core): simplify model-visible context plumbing
Fix stale DeveloperInstructions test usage, deduplicate model/realtime update text logic, add typed no-turn fragment injection, migrate plugin explicit-injection messages off the DeveloperInstructions alias, and streamline initial-context developer fragment assembly. Co-authored-by: Codex <noreply@openai.com>
1 parent 51c06c6 commit b5d8350

File tree

8 files changed

+151
-139
lines changed

8 files changed

+151
-139
lines changed

codex-rs/core/src/agent/control.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use crate::error::CodexErr;
77
use crate::error::Result as CodexResult;
88
use crate::find_thread_path_by_id_str;
99
use crate::rollout::RolloutRecorder;
10+
use crate::session_prefix::SubagentNotification;
1011
use crate::session_prefix::format_subagent_context_line;
11-
use crate::session_prefix::format_subagent_notification_message;
1212
use crate::shell_snapshot::ShellSnapshot;
1313
use crate::state_db;
1414
use crate::thread_manager::ThreadManagerState;
@@ -456,11 +456,12 @@ impl AgentControl {
456456
let Ok(parent_thread) = state.get_thread(parent_thread_id).await else {
457457
return;
458458
};
459+
let child_thread_id_string = child_thread_id.to_string();
459460
parent_thread
460-
.inject_message_without_turn(
461-
MessageRole::Developer,
462-
format_subagent_notification_message(&child_thread_id.to_string(), &status),
463-
)
461+
.inject_model_visible_fragment_without_turn(SubagentNotification::new(
462+
&child_thread_id_string,
463+
&status,
464+
))
464465
.await;
465466
});
466467
}

codex-rs/core/src/codex.rs

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3218,50 +3218,54 @@ impl Session {
32183218
if let Some(developer_instructions) = turn_context.developer_instructions.as_deref() {
32193219
developer_envelope.push(CustomDeveloperInstructions::new(developer_instructions));
32203220
}
3221-
if turn_context.features.enabled(Feature::MemoryTool)
3221+
let memory_prompt = if turn_context.features.enabled(Feature::MemoryTool)
32223222
&& turn_context.config.memories.use_memories
3223-
&& let Some(memory_prompt) =
3224-
build_memory_tool_developer_instructions(&turn_context.config.codex_home).await
32253223
{
3226-
developer_envelope.push(DeveloperTextFragment::new(memory_prompt));
3227-
}
3228-
if let Some(collab_instructions) = developer_collaboration_mode_text(&collaboration_mode) {
3229-
developer_envelope.push(DeveloperTextFragment::new(collab_instructions));
3230-
}
3231-
if let Some(realtime_update) = crate::context_manager::updates::build_realtime_update_item(
3232-
reference_context_item.as_ref(),
3233-
previous_turn_settings.as_ref(),
3234-
turn_context,
3235-
) {
3236-
developer_envelope.push(DeveloperTextFragment::new(realtime_update));
3237-
}
3238-
if self.features.enabled(Feature::Personality)
3239-
&& let Some(personality) = turn_context.personality
3240-
{
3241-
let model_info = turn_context.model_info.clone();
3242-
let has_baked_personality = model_info.supports_personality()
3243-
&& base_instructions == model_info.get_model_instructions(Some(personality));
3244-
if !has_baked_personality
3245-
&& let Some(personality_message) =
3246-
crate::context_manager::updates::personality_message_for(
3247-
&model_info,
3248-
personality,
3224+
build_memory_tool_developer_instructions(&turn_context.config.codex_home).await
3225+
} else {
3226+
None
3227+
};
3228+
let personality_message = if self.features.enabled(Feature::Personality) {
3229+
turn_context.personality.and_then(|personality| {
3230+
let model_info = turn_context.model_info.clone();
3231+
let has_baked_personality = model_info.supports_personality()
3232+
&& base_instructions == model_info.get_model_instructions(Some(personality));
3233+
if has_baked_personality {
3234+
return None;
3235+
}
3236+
crate::context_manager::updates::personality_message_for(&model_info, personality)
3237+
.map(developer_personality_spec_text)
3238+
})
3239+
} else {
3240+
None
3241+
};
3242+
for developer_text in [
3243+
memory_prompt,
3244+
developer_collaboration_mode_text(&collaboration_mode),
3245+
crate::context_manager::updates::build_realtime_update_item(
3246+
reference_context_item.as_ref(),
3247+
previous_turn_settings.as_ref(),
3248+
turn_context,
3249+
),
3250+
personality_message,
3251+
turn_context
3252+
.features
3253+
.enabled(Feature::Apps)
3254+
.then(render_apps_section),
3255+
turn_context
3256+
.features
3257+
.enabled(Feature::CodexGitCommit)
3258+
.then(|| {
3259+
commit_message_trailer_instruction(
3260+
turn_context.config.commit_attribution.as_deref(),
32493261
)
3250-
{
3251-
developer_envelope.push(DeveloperTextFragment::new(
3252-
developer_personality_spec_text(personality_message),
3253-
));
3254-
}
3255-
}
3256-
if turn_context.features.enabled(Feature::Apps) {
3257-
developer_envelope.push(DeveloperTextFragment::new(render_apps_section()));
3258-
}
3259-
if turn_context.features.enabled(Feature::CodexGitCommit)
3260-
&& let Some(commit_message_instruction) = commit_message_trailer_instruction(
3261-
turn_context.config.commit_attribution.as_deref(),
3262-
)
3262+
})
3263+
.flatten(),
3264+
]
3265+
.into_iter()
3266+
.flatten()
32633267
{
3264-
developer_envelope.push(DeveloperTextFragment::new(commit_message_instruction));
3268+
developer_envelope.push(DeveloperTextFragment::new(developer_text));
32653269
}
32663270
if let Some(user_instructions) =
32673271
<AgentsMdInstructions as TurnContextDiffFragment>::from_turn_context(

codex-rs/core/src/codex_tests.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use crate::config_loader::Sourced;
1010
use crate::exec::ExecToolCallOutput;
1111
use crate::function_tool::FunctionCallError;
1212
use crate::mcp_connection_manager::ToolInfo;
13+
use crate::model_visible_context::DeveloperTextFragment;
14+
use crate::model_visible_context::ModelVisibleContextFragment;
1315
use crate::models_manager::model_info;
1416
use crate::shell::default_user_shell;
1517
use crate::tools::format_exec_output_str;
@@ -52,6 +54,7 @@ use codex_protocol::models::BaseInstructions;
5254
use codex_protocol::models::ContentItem;
5355
use codex_protocol::models::ResponseInputItem;
5456
use codex_protocol::models::ResponseItem;
57+
use codex_protocol::models::developer_personality_spec_text;
5558
use codex_protocol::openai_models::ModelsResponse;
5659
use codex_protocol::protocol::ConversationAudioParams;
5760
use codex_protocol::protocol::RealtimeAudioFrame;
@@ -3625,7 +3628,8 @@ async fn sample_rollout(
36253628
.as_ref()
36263629
.and_then(|m| m.get_personality_message(Some(p)).filter(|s| !s.is_empty()))
36273630
{
3628-
let msg = DeveloperInstructions::personality_spec_message(personality_message).into();
3631+
let msg = DeveloperTextFragment::new(developer_personality_spec_text(personality_message))
3632+
.into_message();
36293633
let insert_at = initial_context
36303634
.iter()
36313635
.position(|m| matches!(m, ResponseItem::Message { role, .. } if role == "developer"))

codex-rs/core/src/codex_thread.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use crate::config::ConstraintResult;
55
use crate::error::Result as CodexResult;
66
use crate::features::Feature;
77
use crate::file_watcher::WatchRegistration;
8+
use crate::model_visible_context::ModelVisibleContextFragment;
89
use crate::protocol::Event;
910
use crate::protocol::Op;
1011
use crate::protocol::Submission;
@@ -102,16 +103,27 @@ impl CodexThread {
102103
self.codex.session.total_token_usage().await
103104
}
104105

106+
pub(crate) async fn inject_model_visible_fragment_without_turn(
107+
&self,
108+
fragment: impl ModelVisibleContextFragment,
109+
) {
110+
self.inject_response_input_item_without_turn(fragment.into_response_input_item())
111+
.await;
112+
}
113+
105114
pub(crate) async fn inject_message_without_turn(&self, role: MessageRole, message: String) {
106-
let pending_item = ResponseInputItem::Message {
115+
self.inject_response_input_item_without_turn(ResponseInputItem::Message {
107116
role: role.to_string(),
108117
content: vec![ContentItem::InputText { text: message }],
109-
};
110-
let pending_items = vec![pending_item];
118+
})
119+
.await;
120+
}
121+
122+
async fn inject_response_input_item_without_turn(&self, pending_item: ResponseInputItem) {
111123
let Err(items_without_active_turn) = self
112124
.codex
113125
.session
114-
.inject_response_items(pending_items)
126+
.inject_response_items(vec![pending_item])
115127
.await
116128
else {
117129
return;

codex-rs/core/src/context_manager/updates/developer_update_fragments.rs

Lines changed: 62 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,40 @@ use codex_protocol::models::developer_realtime_start_text;
2323
use codex_protocol::openai_models::ModelInfo;
2424
use codex_protocol::protocol::TurnContextItem;
2525

26+
fn model_instructions_update_text(
27+
previous_model: Option<&str>,
28+
turn_context: &TurnContext,
29+
) -> Option<String> {
30+
if previous_model == Some(turn_context.model_info.slug.as_str()) {
31+
return None;
32+
}
33+
34+
let model_instructions = turn_context
35+
.model_info
36+
.get_model_instructions(turn_context.personality);
37+
if model_instructions.is_empty() {
38+
return None;
39+
}
40+
41+
Some(developer_model_switch_text(model_instructions))
42+
}
43+
44+
fn realtime_update_text(
45+
previous_realtime_active: Option<bool>,
46+
current_realtime_active: bool,
47+
previous_turn_settings: Option<&PreviousTurnSettings>,
48+
) -> Option<String> {
49+
match (previous_realtime_active, current_realtime_active) {
50+
(Some(true), false) => Some(developer_realtime_end_text("inactive")),
51+
(Some(false), true) | (None, true) => Some(developer_realtime_start_text()),
52+
(Some(true), true) | (Some(false), false) => None,
53+
(None, false) => previous_turn_settings
54+
.and_then(|settings| settings.realtime_active)
55+
.filter(|realtime_active| *realtime_active)
56+
.map(|_| developer_realtime_end_text("inactive")),
57+
}
58+
}
59+
2660
// ---------------------------------------------------------------------------
2761
// Settings-update fragments for the developer envelope
2862
// ---------------------------------------------------------------------------
@@ -140,42 +174,25 @@ impl TurnContextDiffFragment for RealtimeUpdateFragment {
140174
turn_context: &TurnContext,
141175
params: &TurnContextDiffParams<'_>,
142176
) -> Option<Self> {
143-
if turn_context.realtime_active {
144-
return Some(Self {
145-
text: developer_realtime_start_text(),
146-
});
147-
}
148-
149-
params
150-
.previous_turn_settings
151-
.and_then(|settings| settings.realtime_active)
152-
.filter(|realtime_active| *realtime_active)
153-
.map(|_| Self {
154-
text: developer_realtime_end_text("inactive"),
155-
})
177+
realtime_update_text(
178+
None,
179+
turn_context.realtime_active,
180+
params.previous_turn_settings,
181+
)
182+
.map(|text| Self { text })
156183
}
157184

158185
fn diff_from_turn_context_item(
159186
previous: &TurnContextItem,
160187
turn_context: &TurnContext,
161188
params: &TurnContextDiffParams<'_>,
162189
) -> Option<Self> {
163-
match (previous.realtime_active, turn_context.realtime_active) {
164-
(Some(true), false) => Some(Self {
165-
text: developer_realtime_end_text("inactive"),
166-
}),
167-
(Some(false), true) | (None, true) => Some(Self {
168-
text: developer_realtime_start_text(),
169-
}),
170-
(Some(true), true) | (Some(false), false) => None,
171-
(None, false) => params
172-
.previous_turn_settings
173-
.and_then(|settings| settings.realtime_active)
174-
.filter(|realtime_active| *realtime_active)
175-
.map(|_| Self {
176-
text: developer_realtime_end_text("inactive"),
177-
}),
178-
}
190+
realtime_update_text(
191+
previous.realtime_active,
192+
turn_context.realtime_active,
193+
params.previous_turn_settings,
194+
)
195+
.map(|text| Self { text })
179196
}
180197
}
181198

@@ -250,42 +267,22 @@ impl TurnContextDiffFragment for ModelInstructionsUpdateFragment {
250267
turn_context: &TurnContext,
251268
params: &TurnContextDiffParams<'_>,
252269
) -> Option<Self> {
253-
let previous_turn_settings = params.previous_turn_settings?;
254-
if previous_turn_settings.model == turn_context.model_info.slug {
255-
return None;
256-
}
257-
258-
let model_instructions = turn_context
259-
.model_info
260-
.get_model_instructions(turn_context.personality);
261-
if model_instructions.is_empty() {
262-
return None;
263-
}
264-
265-
Some(Self {
266-
text: developer_model_switch_text(model_instructions),
267-
})
270+
model_instructions_update_text(
271+
params
272+
.previous_turn_settings
273+
.map(|settings| settings.model.as_str()),
274+
turn_context,
275+
)
276+
.map(|text| Self { text })
268277
}
269278

270279
fn diff_from_turn_context_item(
271280
previous: &TurnContextItem,
272281
turn_context: &TurnContext,
273282
_params: &TurnContextDiffParams<'_>,
274283
) -> Option<Self> {
275-
if previous.model == turn_context.model_info.slug {
276-
return None;
277-
}
278-
279-
let model_instructions = turn_context
280-
.model_info
281-
.get_model_instructions(turn_context.personality);
282-
if model_instructions.is_empty() {
283-
return None;
284-
}
285-
286-
Some(Self {
287-
text: developer_model_switch_text(model_instructions),
288-
})
284+
model_instructions_update_text(Some(previous.model.as_str()), turn_context)
285+
.map(|text| Self { text })
289286
}
290287
}
291288

@@ -359,36 +356,20 @@ pub(crate) fn build_model_instructions_update_item(
359356
previous_turn_settings: Option<&PreviousTurnSettings>,
360357
turn_context: &TurnContext,
361358
) -> Option<String> {
362-
let previous_turn_settings = previous_turn_settings?;
363-
if previous_turn_settings.model == turn_context.model_info.slug {
364-
return None;
365-
}
366-
367-
let model_instructions = turn_context
368-
.model_info
369-
.get_model_instructions(turn_context.personality);
370-
if model_instructions.is_empty() {
371-
return None;
372-
}
373-
374-
Some(developer_model_switch_text(model_instructions))
359+
model_instructions_update_text(
360+
previous_turn_settings.map(|settings| settings.model.as_str()),
361+
turn_context,
362+
)
375363
}
376364

377365
pub(crate) fn build_realtime_update_item(
378366
previous: Option<&TurnContextItem>,
379367
previous_turn_settings: Option<&PreviousTurnSettings>,
380368
turn_context: &TurnContext,
381369
) -> Option<String> {
382-
match (
370+
realtime_update_text(
383371
previous.and_then(|item| item.realtime_active),
384372
turn_context.realtime_active,
385-
) {
386-
(Some(true), false) => Some(developer_realtime_end_text("inactive")),
387-
(Some(false), true) | (None, true) => Some(developer_realtime_start_text()),
388-
(Some(true), true) | (Some(false), false) => None,
389-
(None, false) => previous_turn_settings
390-
.and_then(|settings| settings.realtime_active)
391-
.filter(|realtime_active| *realtime_active)
392-
.map(|_| developer_realtime_end_text("inactive")),
393-
}
373+
previous_turn_settings,
374+
)
394375
}

codex-rs/core/src/model_visible_context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ pub(crate) trait ModelVisibleContextFragment {
125125
{
126126
model_visible_message::<Self::Role>(self.render_text())
127127
}
128+
129+
fn into_response_input_item(self) -> ResponseInputItem
130+
where
131+
Self: Sized,
132+
{
133+
model_visible_response_input_item::<Self::Role>(self.render_text())
134+
}
128135
}
129136

130137
pub(crate) struct DeveloperTextFragment {

0 commit comments

Comments
 (0)