Skip to content

Commit 0e373a6

Browse files
committed
Fix contextual metadata fallbacks
Skip contextual user response items in metadata and rollout fallback scanning, and truncate code-mode notify output before preserving it.
1 parent d91a3b8 commit 0e373a6

8 files changed

Lines changed: 231 additions & 114 deletions

File tree

codex-rs/core/src/context/contextual_user_message.rs

Lines changed: 3 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,10 @@
11
use codex_protocol::items::HookPromptItem;
2+
use codex_protocol::items::is_contextual_user_fragment as protocol_is_contextual_user_fragment;
23
use codex_protocol::items::parse_hook_prompt_fragment;
34
use codex_protocol::models::ContentItem;
45

5-
use super::AdditionalContextUserFragment;
6-
use super::EnvironmentContext;
7-
use super::FragmentRegistration;
8-
use super::FragmentRegistrationProxy;
9-
use super::GoalContext;
10-
use super::LegacyApplyPatchExecCommandWarning;
11-
use super::LegacyModelMismatchWarning;
12-
use super::LegacyUnifiedExecProcessLimitWarning;
13-
use super::SkillInstructions;
14-
use super::SubagentNotification;
15-
use super::TurnAborted;
16-
use super::UserInstructions;
17-
use super::UserShellCommand;
18-
19-
static USER_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy<UserInstructions> =
20-
FragmentRegistrationProxy::new();
21-
static ENVIRONMENT_CONTEXT_REGISTRATION: FragmentRegistrationProxy<EnvironmentContext> =
22-
FragmentRegistrationProxy::new();
23-
static ADDITIONAL_CONTEXT_REGISTRATION: FragmentRegistrationProxy<AdditionalContextUserFragment> =
24-
FragmentRegistrationProxy::new();
25-
static SKILL_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy<SkillInstructions> =
26-
FragmentRegistrationProxy::new();
27-
static USER_SHELL_COMMAND_REGISTRATION: FragmentRegistrationProxy<UserShellCommand> =
28-
FragmentRegistrationProxy::new();
29-
static TURN_ABORTED_REGISTRATION: FragmentRegistrationProxy<TurnAborted> =
30-
FragmentRegistrationProxy::new();
31-
static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy<SubagentNotification> =
32-
FragmentRegistrationProxy::new();
33-
static GOAL_CONTEXT_REGISTRATION: FragmentRegistrationProxy<GoalContext> =
34-
FragmentRegistrationProxy::new();
35-
static LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION: FragmentRegistrationProxy<
36-
LegacyUnifiedExecProcessLimitWarning,
37-
> = FragmentRegistrationProxy::new();
38-
static LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION: FragmentRegistrationProxy<
39-
LegacyApplyPatchExecCommandWarning,
40-
> = FragmentRegistrationProxy::new();
41-
static LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION: FragmentRegistrationProxy<
42-
LegacyModelMismatchWarning,
43-
> = FragmentRegistrationProxy::new();
44-
45-
static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[
46-
&USER_INSTRUCTIONS_REGISTRATION,
47-
&ENVIRONMENT_CONTEXT_REGISTRATION,
48-
&ADDITIONAL_CONTEXT_REGISTRATION,
49-
&SKILL_INSTRUCTIONS_REGISTRATION,
50-
&USER_SHELL_COMMAND_REGISTRATION,
51-
&TURN_ABORTED_REGISTRATION,
52-
&SUBAGENT_NOTIFICATION_REGISTRATION,
53-
&GOAL_CONTEXT_REGISTRATION,
54-
&LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION,
55-
&LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION,
56-
&LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION,
57-
];
58-
59-
fn is_standard_contextual_user_text(text: &str) -> bool {
60-
CONTEXTUAL_USER_FRAGMENTS
61-
.iter()
62-
.any(|fragment| fragment.matches_text(text))
63-
}
64-
656
pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool {
66-
let ContentItem::InputText { text } = content_item else {
67-
return false;
68-
};
69-
parse_hook_prompt_fragment(text).is_some() || is_standard_contextual_user_text(text)
7+
protocol_is_contextual_user_fragment(content_item)
708
}
719

7210
pub(crate) fn parse_visible_hook_prompt_message(
@@ -83,7 +21,7 @@ pub(crate) fn parse_visible_hook_prompt_message(
8321
fragments.push(fragment);
8422
continue;
8523
}
86-
if is_standard_contextual_user_text(text) {
24+
if is_contextual_user_fragment(content_item) {
8725
continue;
8826
}
8927
return None;

codex-rs/core/src/context/fragment.rs

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,6 @@
11
use codex_protocol::models::ContentItem;
22
use codex_protocol::models::ResponseInputItem;
33
use codex_protocol::models::ResponseItem;
4-
use std::marker::PhantomData;
5-
6-
/// Type-erased registration for a contextual user fragment.
7-
///
8-
/// Implementations are used by context filtering code to recognize injected
9-
/// fragments without constructing the concrete context payload.
10-
pub(crate) trait FragmentRegistration: Sync {
11-
fn matches_text(&self, text: &str) -> bool;
12-
}
13-
14-
pub(crate) struct FragmentRegistrationProxy<T> {
15-
_marker: PhantomData<fn() -> T>,
16-
}
17-
18-
impl<T> FragmentRegistrationProxy<T> {
19-
pub(crate) const fn new() -> Self {
20-
Self {
21-
_marker: PhantomData,
22-
}
23-
}
24-
}
25-
26-
impl<T: ContextualUserFragment> FragmentRegistration for FragmentRegistrationProxy<T> {
27-
fn matches_text(&self, text: &str) -> bool {
28-
T::matches_text(text)
29-
}
30-
}
314

325
/// Context payload that is injected as a message fragment.
336
///

codex-rs/core/src/context/mod.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ mod goal_context;
1313
mod guardian_followup_review_reminder;
1414
mod hook_additional_context;
1515
mod image_generation_instructions;
16-
mod legacy_apply_patch_exec_command_warning;
17-
mod legacy_model_mismatch_warning;
18-
mod legacy_unified_exec_process_limit_warning;
1916
mod model_switch_instructions;
2017
mod network_rule_saved;
2118
mod permissions_instructions;
@@ -39,17 +36,12 @@ pub(crate) use contextual_user_message::is_contextual_user_fragment;
3936
pub(crate) use contextual_user_message::parse_visible_hook_prompt_message;
4037
pub(crate) use environment_context::EnvironmentContext;
4138
pub use fragment::ContextualUserFragment;
42-
pub(crate) use fragment::FragmentRegistration;
43-
pub(crate) use fragment::FragmentRegistrationProxy;
4439
pub(crate) use fragments::AdditionalContextDeveloperFragment;
4540
pub(crate) use fragments::AdditionalContextUserFragment;
4641
pub use goal_context::GoalContext;
4742
pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder;
4843
pub(crate) use hook_additional_context::HookAdditionalContext;
4944
pub(crate) use image_generation_instructions::ImageGenerationInstructions;
50-
pub(crate) use legacy_apply_patch_exec_command_warning::LegacyApplyPatchExecCommandWarning;
51-
pub(crate) use legacy_model_mismatch_warning::LegacyModelMismatchWarning;
52-
pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProcessLimitWarning;
5345
pub(crate) use model_switch_instructions::ModelSwitchInstructions;
5446
pub(crate) use network_rule_saved::NetworkRuleSaved;
5547
pub use permissions_instructions::PermissionsInstructions;

codex-rs/core/src/tools/code_mode/mod.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,16 @@ impl CodeModeTurnHost for CoreTurnHost {
132132
if text.trim().is_empty() {
133133
return Ok(());
134134
}
135+
let output = FunctionCallOutputPayload::from_content_items(truncate_code_mode_result(
136+
vec![FunctionCallOutputContentItem::InputText { text }],
137+
None,
138+
));
135139
self.exec
136140
.session
137141
.inject_response_items(vec![ResponseInputItem::CustomToolCallOutput {
138142
call_id,
139143
name: Some(PUBLIC_TOOL_NAME.to_string()),
140-
output: FunctionCallOutputPayload::from_text(text),
144+
output,
141145
}])
142146
.await
143147
.map_err(|_| {
@@ -328,8 +332,10 @@ fn build_freeform_tool_payload(
328332
#[cfg(test)]
329333
mod tests {
330334
use super::build_nested_tool_payload;
335+
use super::truncate_code_mode_result;
331336
use crate::tools::context::ToolPayload;
332337
use codex_code_mode::CodeModeToolKind;
338+
use codex_protocol::models::FunctionCallOutputContentItem;
333339
use codex_tools::ToolName;
334340
use serde_json::json;
335341

@@ -366,4 +372,24 @@ mod tests {
366372
other => panic!("expected freeform payload, got {other:?}"),
367373
}
368374
}
375+
376+
#[test]
377+
fn truncate_code_mode_result_bounds_text_items() {
378+
let original = "notification output ".repeat(1_000);
379+
let items = truncate_code_mode_result(
380+
vec![FunctionCallOutputContentItem::InputText {
381+
text: original.clone(),
382+
}],
383+
Some(10),
384+
);
385+
386+
let [FunctionCallOutputContentItem::InputText { text }] = items.as_slice() else {
387+
panic!("expected a single text item");
388+
};
389+
assert_ne!(text, &original);
390+
assert!(
391+
text.contains("tokens truncated"),
392+
"expected token truncation marker, got {text}"
393+
);
394+
}
369395
}

codex-rs/protocol/src/items.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,77 @@ use std::path::PathBuf;
3535
use std::time::Duration;
3636
use ts_rs::TS;
3737

38+
const CONTEXTUAL_USER_MARKERS: &[(&str, &str)] = &[
39+
("# AGENTS.md instructions for ", "</INSTRUCTIONS>"),
40+
(
41+
crate::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG,
42+
crate::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG,
43+
),
44+
("<skill>", "</skill>"),
45+
("<user_shell_command>", "</user_shell_command>"),
46+
("<turn_aborted>", "</turn_aborted>"),
47+
("<subagent_notification>", "</subagent_notification>"),
48+
("<goal_context>", "</goal_context>"),
49+
];
50+
51+
const LEGACY_CONTEXTUAL_USER_PREFIXES: &[&str] = &[
52+
"Warning: The maximum number of unified exec processes you can keep open is",
53+
"Warning: Your account was flagged for potentially high-risk cyber activity",
54+
];
55+
56+
pub fn is_contextual_user_message_content(message: &[ContentItem]) -> bool {
57+
message.iter().any(is_contextual_user_fragment)
58+
}
59+
60+
pub fn is_contextual_user_fragment(content_item: &ContentItem) -> bool {
61+
let ContentItem::InputText { text } = content_item else {
62+
return false;
63+
};
64+
parse_hook_prompt_fragment(text).is_some()
65+
|| is_standard_contextual_user_text(text)
66+
|| is_legacy_contextual_user_text(text)
67+
}
68+
69+
fn is_standard_contextual_user_text(text: &str) -> bool {
70+
CONTEXTUAL_USER_MARKERS
71+
.iter()
72+
.any(|(start, end)| matches_marked_text(start, end, text))
73+
|| matches_additional_context_user_text(text)
74+
}
75+
76+
fn matches_marked_text(start_marker: &str, end_marker: &str, text: &str) -> bool {
77+
let trimmed_start = text.trim_start();
78+
let starts_with_marker = trimmed_start
79+
.get(..start_marker.len())
80+
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(start_marker));
81+
let trimmed = trimmed_start.trim_end();
82+
let ends_with_marker = trimmed
83+
.get(trimmed.len().saturating_sub(end_marker.len())..)
84+
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(end_marker));
85+
starts_with_marker && ends_with_marker
86+
}
87+
88+
fn matches_additional_context_user_text(text: &str) -> bool {
89+
let trimmed = text.trim();
90+
let Some(rest) = trimmed.strip_prefix("<external_") else {
91+
return false;
92+
};
93+
let Some((key, value_and_close)) = rest.split_once('>') else {
94+
return false;
95+
};
96+
97+
value_and_close.ends_with(&format!("</external_{key}>"))
98+
}
99+
100+
fn is_legacy_contextual_user_text(text: &str) -> bool {
101+
let trimmed = text.trim();
102+
LEGACY_CONTEXTUAL_USER_PREFIXES
103+
.iter()
104+
.any(|prefix| trimmed.starts_with(prefix))
105+
|| (trimmed.starts_with("Warning: apply_patch was requested via ")
106+
&& trimmed.ends_with("Use the apply_patch tool instead of exec_command."))
107+
}
108+
38109
#[allow(clippy::large_enum_variant)]
39110
#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)]
40111
#[serde(tag = "type")]
@@ -637,4 +708,22 @@ mod tests {
637708
}
638709
);
639710
}
711+
712+
#[test]
713+
fn detects_contextual_user_message_content() {
714+
let content = vec![ContentItem::InputText {
715+
text: "<environment_context>\n<cwd>/tmp</cwd>\n</environment_context>".to_string(),
716+
}];
717+
718+
assert!(is_contextual_user_message_content(&content));
719+
}
720+
721+
#[test]
722+
fn ignores_regular_user_message_content() {
723+
let content = vec![ContentItem::InputText {
724+
text: "please inspect the diff".to_string(),
725+
}];
726+
727+
assert!(!is_contextual_user_message_content(&content));
728+
}
640729
}

codex-rs/rollout/src/list.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use crate::protocol::EventMsg;
2222
use crate::state_db;
2323
use codex_file_search as file_search;
2424
use codex_protocol::ThreadId;
25+
use codex_protocol::items::is_contextual_user_message_content;
2526
use codex_protocol::models::ContentItem;
2627
use codex_protocol::models::ResponseItem;
2728
use codex_protocol::protocol::RolloutItem;
@@ -1249,6 +1250,9 @@ fn response_item_preview(item: &ResponseItem) -> Option<String> {
12491250
if role != "user" {
12501251
return None;
12511252
}
1253+
if is_contextual_user_message_content(content) {
1254+
return None;
1255+
}
12521256
for item in content {
12531257
if let ContentItem::InputText { text } = item {
12541258
let message = strip_user_message_prefix(text.as_str());

0 commit comments

Comments
 (0)