mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
Propagate skill warning to thread-start clients
This commit is contained in:
@@ -2671,25 +2671,12 @@ impl CodexMessageProcessor {
|
||||
))
|
||||
.await;
|
||||
|
||||
let warning_connection_ids = [response_connection_id];
|
||||
for warning in thread_start_warnings {
|
||||
listener_task_context
|
||||
.outgoing
|
||||
.send_server_notification_to_connections(
|
||||
&warning_connection_ids,
|
||||
ServerNotification::ConfigWarning(ConfigWarningNotification {
|
||||
summary: warning,
|
||||
details: None,
|
||||
path: None,
|
||||
range: None,
|
||||
}),
|
||||
)
|
||||
.instrument(tracing::info_span!(
|
||||
"app_server.thread_start.notify_config_warning",
|
||||
otel.name = "app_server.thread_start.notify_config_warning",
|
||||
))
|
||||
.await;
|
||||
}
|
||||
send_thread_start_warnings_to_connection(
|
||||
&listener_task_context.outgoing,
|
||||
response_connection_id,
|
||||
thread_start_warnings,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(err) => {
|
||||
let error = JSONRPCErrorError {
|
||||
@@ -4701,7 +4688,7 @@ impl CodexMessageProcessor {
|
||||
thread_id,
|
||||
thread: forked_thread,
|
||||
session_configured,
|
||||
..
|
||||
thread_start_warnings,
|
||||
} = match self
|
||||
.thread_manager
|
||||
.fork_thread(
|
||||
@@ -4896,6 +4883,12 @@ impl CodexMessageProcessor {
|
||||
self.outgoing
|
||||
.send_server_notification(ServerNotification::ThreadStarted(notif))
|
||||
.await;
|
||||
send_thread_start_warnings_to_connection(
|
||||
&self.outgoing,
|
||||
connection_id,
|
||||
thread_start_warnings,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn get_thread_summary(
|
||||
@@ -7445,7 +7438,7 @@ impl CodexMessageProcessor {
|
||||
thread_id,
|
||||
thread: review_thread,
|
||||
session_configured,
|
||||
..
|
||||
thread_start_warnings,
|
||||
} = self
|
||||
.thread_manager
|
||||
.fork_thread(
|
||||
@@ -7493,6 +7486,12 @@ impl CodexMessageProcessor {
|
||||
self.outgoing
|
||||
.send_server_notification(ServerNotification::ThreadStarted(notif))
|
||||
.await;
|
||||
send_thread_start_warnings_to_connection(
|
||||
&self.outgoing,
|
||||
request_id.connection_id,
|
||||
thread_start_warnings,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
@@ -8383,6 +8382,31 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
async fn send_thread_start_warnings_to_connection(
|
||||
outgoing: &Arc<OutgoingMessageSender>,
|
||||
connection_id: ConnectionId,
|
||||
thread_start_warnings: Vec<String>,
|
||||
) {
|
||||
let warning_connection_ids = [connection_id];
|
||||
for warning in thread_start_warnings {
|
||||
outgoing
|
||||
.send_server_notification_to_connections(
|
||||
&warning_connection_ids,
|
||||
ServerNotification::ConfigWarning(ConfigWarningNotification {
|
||||
summary: warning,
|
||||
details: None,
|
||||
path: None,
|
||||
range: None,
|
||||
}),
|
||||
)
|
||||
.instrument(tracing::info_span!(
|
||||
"app_server.thread_start.notify_config_warning",
|
||||
otel.name = "app_server.thread_start.notify_config_warning",
|
||||
))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_thread_list_cwd_filter(
|
||||
cwd: Option<String>,
|
||||
) -> Result<Option<PathBuf>, JSONRPCErrorError> {
|
||||
|
||||
@@ -188,6 +188,72 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_emits_skill_metadata_budget_warning() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
insert_model_context_window(codex_home.path(), 100)?;
|
||||
for i in 0..120 {
|
||||
write_skill(codex_home.path(), &format!("demo-{i:03}"))?;
|
||||
}
|
||||
|
||||
let conversation_id = create_fake_rollout(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
"2025-01-05T12:00:00Z",
|
||||
"Saved user message",
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: conversation_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let fork_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse { thread, .. } = to_response::<ThreadForkResponse>(fork_resp)?;
|
||||
|
||||
let started = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("thread/started"),
|
||||
)
|
||||
.await??;
|
||||
let started: ThreadStartedNotification =
|
||||
serde_json::from_value(started.params.expect("params must be present"))?;
|
||||
assert_eq!(started.thread.id, thread.id);
|
||||
|
||||
let warning = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("configWarning"),
|
||||
)
|
||||
.await??;
|
||||
let parsed: ServerNotification = warning.try_into()?;
|
||||
let ServerNotification::ConfigWarning(notification) = parsed else {
|
||||
panic!("expected configWarning notification");
|
||||
};
|
||||
|
||||
assert!(
|
||||
notification
|
||||
.summary
|
||||
.contains("Skills list trimmed to fit the metadata budget"),
|
||||
"unexpected warning: {}",
|
||||
notification.summary
|
||||
);
|
||||
assert!(notification.summary.contains("2 tokens"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_emits_restored_token_usage_before_next_turn() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -614,6 +680,33 @@ stream_max_retries = 0
|
||||
)
|
||||
}
|
||||
|
||||
fn insert_model_context_window(
|
||||
codex_home: &Path,
|
||||
model_context_window: i64,
|
||||
) -> std::io::Result<()> {
|
||||
let config_toml = codex_home.join("config.toml");
|
||||
let mut config = std::fs::read_to_string(&config_toml)?;
|
||||
let provider_table = "\n[model_providers.mock_provider]";
|
||||
let Some(provider_table_index) = config.find(provider_table) else {
|
||||
return Err(std::io::Error::other(
|
||||
"mock provider table missing from test config",
|
||||
));
|
||||
};
|
||||
config.insert_str(
|
||||
provider_table_index,
|
||||
&format!("\nmodel_context_window = {model_context_window}\n"),
|
||||
);
|
||||
std::fs::write(config_toml, config)
|
||||
}
|
||||
|
||||
fn write_skill(codex_home: &Path, name: &str) -> std::io::Result<()> {
|
||||
let skill_dir = codex_home.join("skills").join(name);
|
||||
std::fs::create_dir_all(&skill_dir)?;
|
||||
let description = "desc ".repeat(200);
|
||||
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
|
||||
std::fs::write(skill_dir.join("SKILL.md"), content)
|
||||
}
|
||||
|
||||
fn create_config_toml_with_chatgpt_base_url(
|
||||
codex_home: &Path,
|
||||
server_uri: &str,
|
||||
|
||||
@@ -5,7 +5,7 @@ use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_output_truncation::approx_token_count;
|
||||
|
||||
pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
|
||||
pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 1;
|
||||
pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
|
||||
const DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT: usize = 5;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@@ -283,10 +283,10 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_budget_uses_one_percent_of_full_context_window() {
|
||||
fn default_budget_uses_two_percent_of_full_context_window() {
|
||||
assert_eq!(
|
||||
default_skill_metadata_budget(Some(200_000)),
|
||||
SkillMetadataBudget::Tokens(2_000)
|
||||
SkillMetadataBudget::Tokens(4_000)
|
||||
);
|
||||
assert_eq!(
|
||||
default_skill_metadata_budget(Some(99)),
|
||||
|
||||
Reference in New Issue
Block a user