mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Support model catalog Apps usage instructions (#61213)
## What changed - Add the disabled-by-default `catalog_apps_instructions` feature to use the selected model's `model_messages.apps.usage_instructions`, subject to existing Apps availability and model gates. - Preserve valid catalog text literally, allow an empty string to clear guidance, and fall back to bundled instructions for missing, whitespace-only, or oversized values. Overrides are limited to 800 UTF-8 bytes. - Reconcile guidance with the latest retained Apps fragment, emitting replacement or removal notices when needed and avoiding repeated updates across resumes. Preserve compatibility with bundled guidance and legacy checkpoints. - Recognize catalog Apps fragments as contextual developer messages in event mapping and rollout rollback. ## Testing Add resolver, world-state, client, and scenario coverage for literal text and byte limits, feature and availability gates, model switches, clearing guidance, resumes, and legacy history recovery. GitOrigin-RevId: 6240d407ada8f22fa101753ff3ddb338632ae283
This commit is contained in:
goodman-oai
@@ -1,6 +1,11 @@
|
||||
//! Renders bundled Apps guidance and catalog updates with the same developer provenance.
|
||||
//! Older clients recognize only the bundled markers when recovering from retained history.
|
||||
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_protocol::protocol::APPS_INSTRUCTIONS_CLOSE_TAG;
|
||||
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::protocol::CATALOG_APPS_INSTRUCTIONS_CLOSE_TAG;
|
||||
use codex_protocol::protocol::CATALOG_APPS_INSTRUCTIONS_OPEN_TAG;
|
||||
|
||||
use super::ContextualUserFragment;
|
||||
use codex_protocol::models::ContentItem;
|
||||
@@ -8,7 +13,17 @@ use codex_protocol::models::ContentItemKind;
|
||||
use codex_protocol::models::input_text;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct AppsInstructions;
|
||||
pub(crate) struct AppsInstructions(pub(crate) String);
|
||||
|
||||
impl Default for AppsInstructions {
|
||||
fn default() -> Self {
|
||||
Self(
|
||||
codex_prompts::ResolvedModelMessages::bundled()
|
||||
.apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.usage_instructions,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for AppsInstructions {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
@@ -28,8 +43,34 @@ impl ContextualUserFragment for AppsInstructions {
|
||||
}
|
||||
|
||||
fn body(&self) -> ContentItem {
|
||||
input_text(format!(
|
||||
"\n## Apps (Connectors)\nApps (Connectors) can be explicitly triggered in user messages in the format `[$app-name](app://{{connector_id}})`. Apps can also be implicitly triggered as long as the context suggests usage of available apps.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nAn installed app's MCP tools are either provided to you already, or can be lazy-loaded through the `tool_search` tool. If `tool_search` is available, the apps that are searchable by `tools_search` will be listed by it.\nDo not additionally call list_mcp_resources or list_mcp_resource_templates for apps.\n"
|
||||
))
|
||||
input_text(self.0.clone())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct CatalogAppsInstructions(pub(crate) String);
|
||||
|
||||
impl ContextualUserFragment for CatalogAppsInstructions {
|
||||
fn content_kind(&self) -> ContentItemKind {
|
||||
ContentItemKind("apps.instructions".to_string())
|
||||
}
|
||||
|
||||
fn role(&self) -> &'static str {
|
||||
"developer"
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
(
|
||||
CATALOG_APPS_INSTRUCTIONS_OPEN_TAG,
|
||||
CATALOG_APPS_INSTRUCTIONS_CLOSE_TAG,
|
||||
)
|
||||
}
|
||||
|
||||
fn body(&self) -> ContentItem {
|
||||
input_text(self.0.clone())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -60,6 +60,7 @@ pub(crate) use approved_command_prefix_saved::APPROVED_COMMAND_PREFIX_SAVED_MESS
|
||||
pub(crate) use approved_command_prefix_saved::ApprovedCommandPrefixSaved;
|
||||
pub(crate) use approved_command_prefix_saved::SAVED_PREFIX_KIND;
|
||||
pub(crate) use apps_instructions::AppsInstructions;
|
||||
pub(crate) use apps_instructions::CatalogAppsInstructions;
|
||||
pub(crate) use available_plugins_instructions::AvailablePluginsInstructions;
|
||||
pub(crate) use base_instructions::BaseInstructionsFragment;
|
||||
pub(crate) use codex_context_fragments::AdditionalContextDeveloperFragment;
|
||||
|
||||
@@ -1,28 +1,133 @@
|
||||
//! Reconciles the newest retained Apps fragment with the selected model's wording.
|
||||
//! The persisted availability bit lets older clients restore their bundled guidance.
|
||||
|
||||
use super::PreviousSectionState;
|
||||
use super::SectionTransition;
|
||||
use super::WorldStateHash;
|
||||
use super::WorldStateSection;
|
||||
use super::WorldStateUpdate;
|
||||
use crate::context::AppsInstructions;
|
||||
use crate::context::CatalogAppsInstructions;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use codex_prompts::ResolvedAppsInstructions;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
/// Whether generic Apps usage guidance should be visible to the model.
|
||||
#[derive(Clone, Copy, Debug, Default)]
|
||||
const REPLACEMENT_NOTICE: &str =
|
||||
"\nThese Apps usage instructions replace all previously provided Apps usage instructions.\n\n";
|
||||
const REMOVAL_NOTICE: &str = "\nThe previously provided Apps usage instructions no longer apply.\n";
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct AppsInstructionsState {
|
||||
available: bool,
|
||||
resolved: ResolvedAppsInstructions,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(untagged)]
|
||||
pub(crate) enum AppsInstructionsSnapshot {
|
||||
/// The pre-catalog reader appends its bundled instructions on false.
|
||||
BundledAvailable(bool),
|
||||
/// Read existing catalog checkpoints. We still use this form when catalog wording is
|
||||
/// byte-identical to bundled wording: an old reader can keep it, while a new reader
|
||||
/// must remember its catalog origin if Apps become unavailable.
|
||||
Catalog {
|
||||
instructions: Option<WorldStateHash>,
|
||||
catalog: bool,
|
||||
},
|
||||
}
|
||||
|
||||
/// The instructions that still apply in the retained history.
|
||||
enum RetainedGuidance<'a> {
|
||||
None,
|
||||
Bundled,
|
||||
Catalog(&'a str),
|
||||
}
|
||||
|
||||
impl<'a> RetainedGuidance<'a> {
|
||||
fn recover(
|
||||
previous: PreviousSectionState<'_, AppsInstructionsSnapshot>,
|
||||
retained: Option<&'a str>,
|
||||
bundled: &'a str,
|
||||
catalog_selected: bool,
|
||||
) -> Self {
|
||||
use AppsInstructionsSnapshot::BundledAvailable;
|
||||
use AppsInstructionsSnapshot::Catalog;
|
||||
|
||||
// The World State matcher has already checked the developer role and both markers.
|
||||
let body = retained.map(str::trim).map(|text| {
|
||||
let (open, close) = if CatalogAppsInstructions::matches_text(text) {
|
||||
CatalogAppsInstructions::type_markers()
|
||||
} else {
|
||||
AppsInstructions::type_markers()
|
||||
};
|
||||
&text[open.len()..text.len() - close.len()]
|
||||
});
|
||||
let Some(body) = body else {
|
||||
return match previous {
|
||||
PreviousSectionState::Absent
|
||||
| PreviousSectionState::Known(BundledAvailable(false)) => Self::None,
|
||||
PreviousSectionState::Unknown
|
||||
| PreviousSectionState::Known(BundledAvailable(true)) => Self::Bundled,
|
||||
PreviousSectionState::Known(Catalog {
|
||||
instructions: None, ..
|
||||
}) => Self::None,
|
||||
PreviousSectionState::Known(Catalog {
|
||||
instructions: Some(_),
|
||||
..
|
||||
}) => Self::Bundled,
|
||||
};
|
||||
};
|
||||
// Prior catalog checkpoints carried a hash to distinguish literal notice-like content.
|
||||
let saved_literal = matches!(previous, PreviousSectionState::Known(Catalog { instructions: Some(hash), .. })
|
||||
if (body == REMOVAL_NOTICE || body.starts_with(REPLACEMENT_NOTICE)) && *hash == fingerprint(body));
|
||||
let wording = if saved_literal {
|
||||
body
|
||||
} else if body == REMOVAL_NOTICE {
|
||||
return Self::None;
|
||||
} else {
|
||||
body.strip_prefix(REPLACEMENT_NOTICE).unwrap_or(body)
|
||||
};
|
||||
if wording != bundled {
|
||||
return Self::Catalog(wording);
|
||||
}
|
||||
match previous {
|
||||
PreviousSectionState::Known(Catalog { catalog: true, .. }) => Self::Catalog(wording),
|
||||
// Older clients can disable Apps without appending a retirement notice. Re-enable
|
||||
// their bundled wording normally, but let an explicit catalog choice supersede it.
|
||||
PreviousSectionState::Known(
|
||||
BundledAvailable(false)
|
||||
| Catalog {
|
||||
instructions: None,
|
||||
catalog: false,
|
||||
},
|
||||
) if !catalog_selected => Self::None,
|
||||
_ => Self::Bundled,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn fingerprint(text: &str) -> WorldStateHash {
|
||||
WorldStateHash::from_json(&serde_json::Value::String(text.to_owned()))
|
||||
}
|
||||
|
||||
impl AppsInstructionsState {
|
||||
pub(crate) fn new(available: bool) -> Self {
|
||||
Self { available }
|
||||
pub(crate) fn new(available: bool, resolved: ResolvedAppsInstructions) -> Self {
|
||||
Self {
|
||||
available,
|
||||
resolved,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl WorldStateSection for AppsInstructionsState {
|
||||
const ID: &'static str = "apps_instructions";
|
||||
type Snapshot = bool;
|
||||
type Snapshot = AppsInstructionsSnapshot;
|
||||
const NEEDS_LATEST_FRAGMENT: bool = true;
|
||||
|
||||
fn matches_legacy_fragment(role: &str, text: &str) -> bool {
|
||||
role == "developer" && AppsInstructions::matches_text(text)
|
||||
role == "developer"
|
||||
&& (AppsInstructions::matches_text(text) || CatalogAppsInstructions::matches_text(text))
|
||||
}
|
||||
|
||||
fn has_retained_fragment_matcher() -> bool {
|
||||
@@ -37,18 +142,60 @@ impl WorldStateSection for AppsInstructionsState {
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Self::Snapshot>,
|
||||
) -> SectionTransition<Self::Snapshot> {
|
||||
let current = self.available;
|
||||
if !self.available
|
||||
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|
||||
|| matches!(previous, PreviousSectionState::Unknown)
|
||||
{
|
||||
return (Some(current), Vec::new());
|
||||
}
|
||||
self.render_retained_diff(previous, None)
|
||||
}
|
||||
|
||||
(
|
||||
Some(current),
|
||||
vec![WorldStateUpdate::fragment(AppsInstructions)],
|
||||
)
|
||||
fn render_retained_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Self::Snapshot>,
|
||||
retained: Option<&str>,
|
||||
) -> SectionTransition<Self::Snapshot> {
|
||||
let bundled = AppsInstructions::default().0;
|
||||
let catalog = self.resolved.catalog_override;
|
||||
let previous = RetainedGuidance::recover(previous, retained, &bundled, catalog);
|
||||
let instructions = self
|
||||
.available
|
||||
.then_some(self.resolved.usage_instructions.as_str())
|
||||
.filter(|text| !text.is_empty());
|
||||
let is_bundled = instructions == Some(bundled.as_str());
|
||||
let snapshot = if catalog && is_bundled {
|
||||
AppsInstructionsSnapshot::Catalog {
|
||||
instructions: Some(fingerprint(&bundled)),
|
||||
catalog: true,
|
||||
}
|
||||
} else {
|
||||
AppsInstructionsSnapshot::BundledAvailable(is_bundled)
|
||||
};
|
||||
let previous_wording = match previous {
|
||||
RetainedGuidance::None => None,
|
||||
RetainedGuidance::Bundled => Some(bundled.as_str()),
|
||||
RetainedGuidance::Catalog(text) => Some(text),
|
||||
};
|
||||
if previous_wording == instructions {
|
||||
return (Some(snapshot), Vec::new());
|
||||
}
|
||||
let body = match instructions {
|
||||
// A notice-like literal must have a replacement prefix even on its first appearance,
|
||||
// so recovery can strip exactly one layer and keep every byte of the supplied text.
|
||||
Some(text)
|
||||
if previous_wording.is_some()
|
||||
|| text.starts_with(REPLACEMENT_NOTICE)
|
||||
|| text == REMOVAL_NOTICE =>
|
||||
{
|
||||
format!("{REPLACEMENT_NOTICE}{text}")
|
||||
}
|
||||
Some(text) => text.to_owned(),
|
||||
None if catalog || matches!(previous, RetainedGuidance::Catalog(_)) => {
|
||||
REMOVAL_NOTICE.to_owned()
|
||||
}
|
||||
None => return (Some(snapshot), Vec::new()),
|
||||
};
|
||||
let update = if is_bundled {
|
||||
WorldStateUpdate::fragment(AppsInstructions(body))
|
||||
} else {
|
||||
WorldStateUpdate::fragment(CatalogAppsInstructions(body))
|
||||
};
|
||||
(Some(snapshot), vec![update])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,9 +1,14 @@
|
||||
#![cfg(test)]
|
||||
|
||||
use super::*;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::world_state::PreviousSectionState;
|
||||
use crate::context::world_state::WorldState;
|
||||
use crate::context::world_state::WorldStateSnapshot;
|
||||
use crate::context::world_state::test_support::render_section_cases;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_prompts::ResolvedModelMessages;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn snapshots() {
|
||||
@@ -11,8 +16,10 @@ fn snapshots() {
|
||||
use PreviousSectionState::Known;
|
||||
use PreviousSectionState::Unknown;
|
||||
|
||||
let unavailable = AppsInstructionsState::new(/*available*/ false);
|
||||
let available = AppsInstructionsState::new(/*available*/ true);
|
||||
let bundled =
|
||||
ResolvedModelMessages::bundled().apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME);
|
||||
let unavailable = AppsInstructionsState::new(/*available*/ false, bundled.clone());
|
||||
let available = AppsInstructionsState::new(/*available*/ true, bundled);
|
||||
|
||||
insta::assert_snapshot!(render_section_cases(&[
|
||||
(Absent, Absent),
|
||||
@@ -27,37 +34,411 @@ fn snapshots() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_guidance_is_not_injected_again() {
|
||||
let mut world_state = super::super::WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(/*available*/ true));
|
||||
let legacy: ResponseItem = ContextualUserFragment::into(AppsInstructions);
|
||||
fn legacy_guidance_is_reused_or_retired_by_catalog_intent() {
|
||||
let bundled = AppsInstructions::default().0;
|
||||
let replacement = format!("{REPLACEMENT_NOTICE}{bundled}");
|
||||
let custom = format!("{REPLACEMENT_NOTICE}B");
|
||||
let removal = REMOVAL_NOTICE.to_owned();
|
||||
for (saved_available, retained, catalog, available, expected) in [
|
||||
(None, bundled.as_str(), None, true, None),
|
||||
(Some(true), bundled.as_str(), None, true, None),
|
||||
(
|
||||
Some(false),
|
||||
bundled.as_str(),
|
||||
Some("B"),
|
||||
false,
|
||||
Some(removal.as_str()),
|
||||
),
|
||||
(
|
||||
Some(false),
|
||||
bundled.as_str(),
|
||||
Some(""),
|
||||
false,
|
||||
Some(removal.as_str()),
|
||||
),
|
||||
// Older clients can save false while retaining custom instructions from a newer client.
|
||||
(Some(false), "B", None, true, Some(replacement.as_str())),
|
||||
(Some(false), "B", None, false, Some(removal.as_str())),
|
||||
(
|
||||
Some(false),
|
||||
custom.as_str(),
|
||||
None,
|
||||
true,
|
||||
Some(replacement.as_str()),
|
||||
),
|
||||
(
|
||||
Some(false),
|
||||
custom.as_str(),
|
||||
None,
|
||||
false,
|
||||
Some(removal.as_str()),
|
||||
),
|
||||
// Bundled-only false snapshots keep their historical availability-cycle behavior.
|
||||
(
|
||||
Some(false),
|
||||
bundled.as_str(),
|
||||
None,
|
||||
true,
|
||||
Some(bundled.as_str()),
|
||||
),
|
||||
(Some(false), bundled.as_str(), None, false, None),
|
||||
(
|
||||
Some(false),
|
||||
replacement.as_str(),
|
||||
None,
|
||||
true,
|
||||
Some(bundled.as_str()),
|
||||
),
|
||||
(Some(false), replacement.as_str(), None, false, None),
|
||||
] {
|
||||
let legacy: ResponseItem =
|
||||
ContextualUserFragment::into(AppsInstructions(retained.to_string()));
|
||||
let mut model = codex_models_manager::model_info::model_info_from_slug("gpt-5.5");
|
||||
model.model_messages =
|
||||
Some(serde_json::from_value(json!({"apps": {"usage_instructions": catalog}})).unwrap());
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
available,
|
||||
ResolvedModelMessages::from_model(&model)
|
||||
.apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME),
|
||||
));
|
||||
let snapshot: Option<WorldStateSnapshot> = saved_available.map(|available| {
|
||||
serde_json::from_value(json!({"apps_instructions": available})).unwrap()
|
||||
});
|
||||
assert_eq!(
|
||||
world_state
|
||||
.render_history_fragment_diff(snapshot.as_ref(), std::slice::from_ref(&legacy))
|
||||
.1
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>(),
|
||||
expected
|
||||
.map(|text| {
|
||||
if text == bundled || text == format!("{REPLACEMENT_NOTICE}{bundled}") {
|
||||
ContextualUserFragment::into(AppsInstructions(text.to_string()))
|
||||
} else {
|
||||
ContextualUserFragment::into(CatalogAppsInstructions(text.to_string()))
|
||||
}
|
||||
})
|
||||
.into_iter()
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_fragment_diff(/*previous*/ None, &[legacy])
|
||||
.1
|
||||
.is_empty()
|
||||
);
|
||||
#[test]
|
||||
fn resume_after_an_older_catalog_client_silently_disabled_bundled_guidance() {
|
||||
let retained = ContextualUserFragment::into(AppsInstructions::default());
|
||||
let previous: WorldStateSnapshot =
|
||||
serde_json::from_value(json!({"apps_instructions": {"catalog": false}})).unwrap();
|
||||
let bundled =
|
||||
ResolvedModelMessages::bundled().apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME);
|
||||
for (resolved, expected) in [
|
||||
(bundled, retained.clone()),
|
||||
(
|
||||
ResolvedAppsInstructions {
|
||||
usage_instructions: String::new(),
|
||||
catalog_override: true,
|
||||
},
|
||||
ContextualUserFragment::into(CatalogAppsInstructions(REMOVAL_NOTICE.to_string())),
|
||||
),
|
||||
] {
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
/*available*/ true, resolved,
|
||||
));
|
||||
let (_, updates) = world_state
|
||||
.render_history_fragment_diff(Some(&previous), std::slice::from_ref(&retained));
|
||||
assert_eq!(
|
||||
updates
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>(),
|
||||
[expected]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persisted_guidance_is_restored_only_when_missing_from_history() {
|
||||
let mut world_state = super::super::WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(/*available*/ true));
|
||||
let snapshot = world_state.render_full().0;
|
||||
let retained: ResponseItem = ContextualUserFragment::into(AppsInstructions);
|
||||
|
||||
assert_eq!(
|
||||
world_state
|
||||
.render_history_fragment_diff(Some(&snapshot), &[])
|
||||
.1
|
||||
.len(),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_fragment_diff(Some(&snapshot), &[retained])
|
||||
.1
|
||||
.is_empty()
|
||||
);
|
||||
let retained: ResponseItem = ContextualUserFragment::into(AppsInstructions::default());
|
||||
let bundled =
|
||||
ResolvedModelMessages::bundled().apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME);
|
||||
for history in [vec![], vec![retained]] {
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
/*available*/ true,
|
||||
bundled.clone(),
|
||||
));
|
||||
let snapshot = world_state.render_full().0;
|
||||
assert_eq!(
|
||||
world_state
|
||||
.render_history_fragment_diff(Some(&snapshot), &history)
|
||||
.1
|
||||
.len(),
|
||||
usize::from(history.is_empty())
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_guidance_updates_clears_and_returns_to_bundled() {
|
||||
let bundled = AppsInstructions::default().0;
|
||||
let replacement = format!("{REPLACEMENT_NOTICE}B");
|
||||
let fallback = format!("{REPLACEMENT_NOTICE}{bundled}");
|
||||
let removal = REMOVAL_NOTICE.to_owned();
|
||||
let mut history = Vec::new();
|
||||
let mut previous = None;
|
||||
for (available, catalog, expected) in [
|
||||
(true, Some("A"), Some("A")),
|
||||
(true, Some("A"), None),
|
||||
(true, Some("B"), Some(replacement.as_str())),
|
||||
(true, Some(""), Some(removal.as_str())),
|
||||
(true, Some(""), None),
|
||||
(true, Some("A"), Some("A")),
|
||||
(true, None, Some(fallback.as_str())),
|
||||
(false, None, None),
|
||||
(true, None, Some(bundled.as_str())),
|
||||
(true, Some("B"), Some(replacement.as_str())),
|
||||
(false, Some("B"), Some(removal.as_str())),
|
||||
(false, Some("B"), None),
|
||||
// Selecting catalog intent while unavailable must retire retained bundled guidance.
|
||||
(true, None, Some(bundled.as_str())),
|
||||
(false, Some("B"), Some(removal.as_str())),
|
||||
(false, Some("B"), None),
|
||||
(true, Some("B"), Some("B")),
|
||||
(true, None, Some(fallback.as_str())),
|
||||
(false, Some(""), Some(removal.as_str())),
|
||||
(true, Some(""), None),
|
||||
// A prior bundled-only unavailable turn must not hide later catalog intent.
|
||||
(true, None, Some(bundled.as_str())),
|
||||
(false, None, None),
|
||||
(false, Some("B"), Some(removal.as_str())),
|
||||
(true, Some("B"), Some("B")),
|
||||
(true, None, Some(fallback.as_str())),
|
||||
(false, None, None),
|
||||
(false, Some(""), Some(removal.as_str())),
|
||||
(true, Some(""), None),
|
||||
] {
|
||||
let mut model = codex_models_manager::model_info::model_info_from_slug("gpt-5.5");
|
||||
model.model_messages =
|
||||
Some(serde_json::from_value(json!({"apps": {"usage_instructions": catalog}})).unwrap());
|
||||
let resolved = ResolvedModelMessages::from_model(&model)
|
||||
.apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME);
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(available, resolved));
|
||||
let (snapshot, fragments) =
|
||||
world_state.render_history_fragment_diff(previous.as_ref(), &history);
|
||||
let updates = fragments
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
updates,
|
||||
expected
|
||||
.map(|text| {
|
||||
if text == bundled || text == format!("{REPLACEMENT_NOTICE}{bundled}") {
|
||||
ContextualUserFragment::into(AppsInstructions(text.to_string()))
|
||||
} else {
|
||||
ContextualUserFragment::into(CatalogAppsInstructions(text.to_string()))
|
||||
}
|
||||
})
|
||||
.into_iter()
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
history.extend(updates);
|
||||
previous = Some(snapshot);
|
||||
}
|
||||
}
|
||||
|
||||
// These are the matching and rendering rules of the released availability-only client.
|
||||
struct PreCatalogApps;
|
||||
|
||||
impl WorldStateSection for PreCatalogApps {
|
||||
const ID: &'static str = "apps_instructions";
|
||||
type Snapshot = bool;
|
||||
|
||||
fn has_retained_fragment_matcher() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn matches_retained_fragment(role: &str, text: &str) -> bool {
|
||||
role == "developer" && AppsInstructions::matches_text(text)
|
||||
}
|
||||
|
||||
fn matches_legacy_fragment(role: &str, text: &str) -> bool {
|
||||
Self::matches_retained_fragment(role, text)
|
||||
}
|
||||
|
||||
fn render_diff(&self, previous: PreviousSectionState<'_, bool>) -> SectionTransition<bool> {
|
||||
let updates = match previous {
|
||||
PreviousSectionState::Unknown | PreviousSectionState::Known(true) => Vec::new(),
|
||||
PreviousSectionState::Absent | PreviousSectionState::Known(false) => {
|
||||
vec![WorldStateUpdate::fragment(AppsInstructions::default())]
|
||||
}
|
||||
};
|
||||
(Some(true), updates)
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn downgrade_restores_bundled_guidance_after_catalog_selection_and_retirement() {
|
||||
let bundled: ResponseItem = ContextualUserFragment::into(AppsInstructions::default());
|
||||
let mut previous = None;
|
||||
let mut catalog_history = Vec::new();
|
||||
let mut older_client = WorldState::default();
|
||||
older_client.add_section(PreCatalogApps);
|
||||
for text in ["custom", ""] {
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
/*available*/ true,
|
||||
ResolvedAppsInstructions {
|
||||
usage_instructions: text.to_string(),
|
||||
catalog_override: true,
|
||||
},
|
||||
));
|
||||
let (snapshot, updates) =
|
||||
world_state.render_history_fragment_diff(previous.as_ref(), &catalog_history);
|
||||
assert_eq!(updates.len(), 1);
|
||||
catalog_history.extend(
|
||||
updates
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item),
|
||||
);
|
||||
let saved = serde_json::to_string(&snapshot).unwrap();
|
||||
let restored: WorldStateSnapshot = serde_json::from_str(&saved).unwrap();
|
||||
let mixed_history = std::iter::once(&bundled).chain(&catalog_history);
|
||||
for (saved, history) in [
|
||||
(Some(&restored), mixed_history.collect::<Vec<_>>()),
|
||||
(None, catalog_history.iter().collect()),
|
||||
] {
|
||||
assert_eq!(
|
||||
older_client
|
||||
.render_history_fragment_diff(saved, history)
|
||||
.1
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>(),
|
||||
std::slice::from_ref(&bundled)
|
||||
);
|
||||
}
|
||||
previous = Some(snapshot);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn literal_catalog_notices_remain_active_guidance_after_snapshot_loss() {
|
||||
for text in [
|
||||
REMOVAL_NOTICE.to_owned(),
|
||||
format!("{REPLACEMENT_NOTICE}literal"),
|
||||
] {
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(
|
||||
/*available*/ true,
|
||||
ResolvedAppsInstructions {
|
||||
usage_instructions: text.clone(),
|
||||
catalog_override: true,
|
||||
},
|
||||
));
|
||||
let (_, first) = world_state.render_full_fragments();
|
||||
let history: Vec<_> = first
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect();
|
||||
assert_eq!(
|
||||
history,
|
||||
[ContextualUserFragment::into(CatalogAppsInstructions(
|
||||
format!("{REPLACEMENT_NOTICE}{text}")
|
||||
))]
|
||||
);
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_fragment_diff(/*previous*/ None, &history)
|
||||
.1
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retained_catalog_guidance_recovers_legacy_snapshots_and_repairs_lost_updates() {
|
||||
let mut history = Vec::new();
|
||||
let mut previous = None;
|
||||
let catalog = |text| ContextualUserFragment::into(CatalogAppsInstructions(text));
|
||||
let replacement = |text: &str| format!("{REPLACEMENT_NOTICE}{text}");
|
||||
let removal = catalog(REMOVAL_NOTICE.to_owned());
|
||||
let two_byte_limit = "é".repeat(400);
|
||||
let four_byte_limit = "\u{10ffff}".repeat(200);
|
||||
let oversized = "é".repeat(500);
|
||||
for (available, text, expected) in [
|
||||
(true, "A", catalog("A".to_string())),
|
||||
(true, "B", catalog(replacement("B"))),
|
||||
(false, "B", removal.clone()),
|
||||
(true, "B\r\n", catalog("B\r\n".to_string())),
|
||||
(true, "B\n", catalog(replacement("B\n"))),
|
||||
(
|
||||
true,
|
||||
two_byte_limit.as_str(),
|
||||
catalog(replacement(&two_byte_limit)),
|
||||
),
|
||||
(
|
||||
true,
|
||||
four_byte_limit.as_str(),
|
||||
catalog(replacement(&four_byte_limit)),
|
||||
),
|
||||
(
|
||||
true,
|
||||
oversized.as_str(),
|
||||
ContextualUserFragment::into(AppsInstructions(replacement(
|
||||
&AppsInstructions::default().0,
|
||||
))),
|
||||
),
|
||||
(true, "", removal),
|
||||
] {
|
||||
let mut model = codex_models_manager::model_info::model_info_from_slug("gpt-5.5");
|
||||
model.model_messages =
|
||||
Some(serde_json::from_value(json!({"apps": {"usage_instructions": text}})).unwrap());
|
||||
let resolved = ResolvedModelMessages::from_model(&model)
|
||||
.apps_usage_instructions(CODEX_APPS_MCP_SERVER_NAME);
|
||||
let mut world_state = WorldState::default();
|
||||
world_state.add_section(AppsInstructionsState::new(available, resolved));
|
||||
let (snapshot, fragments) =
|
||||
world_state.render_history_fragment_diff(previous.as_ref(), &history);
|
||||
for fragment in &fragments {
|
||||
assert!(fragment.render().into_text().unwrap().len() <= 1_000);
|
||||
}
|
||||
let updates = fragments
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(updates, [expected]);
|
||||
for saved in [
|
||||
None,
|
||||
Some(json!({"apps_instructions": true})),
|
||||
Some(serde_json::to_value(&snapshot).unwrap()),
|
||||
] {
|
||||
let saved: Option<WorldStateSnapshot> =
|
||||
saved.map(|value| serde_json::from_value(value).unwrap());
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_fragment_diff(saved.as_ref(), &updates)
|
||||
.1
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
if !history.is_empty() {
|
||||
// The newest update was compacted away; an older marker must not suppress repair.
|
||||
assert_eq!(
|
||||
world_state
|
||||
.render_history_fragment_diff(Some(&snapshot), &history)
|
||||
.1
|
||||
.into_iter()
|
||||
.map(ContextualUserFragment::into_boxed_response_item)
|
||||
.collect::<Vec<_>>(),
|
||||
updates
|
||||
);
|
||||
}
|
||||
history.extend(updates);
|
||||
previous = Some(snapshot);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -171,7 +171,15 @@ trait ErasedWorldStateSection: Send + Sync {
|
||||
|
||||
fn matches_retained_fragment(&self, role: &str, text: &str) -> bool;
|
||||
|
||||
fn render_diff(&self, previous: PreviousSectionState<'_, Value>) -> SectionTransition;
|
||||
fn needs_latest_fragment(&self) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn render_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Value>,
|
||||
retained: Option<&str>,
|
||||
) -> SectionTransition;
|
||||
}
|
||||
|
||||
fn section_snapshot<S: WorldStateSection>(section: &S, snapshot: S::Snapshot) -> Option<Value> {
|
||||
@@ -213,7 +221,15 @@ impl<S: WorldStateSection> ErasedWorldStateSection for S {
|
||||
S::matches_retained_fragment(role, text)
|
||||
}
|
||||
|
||||
fn render_diff(&self, previous: PreviousSectionState<'_, Value>) -> SectionTransition {
|
||||
fn needs_latest_fragment(&self) -> bool {
|
||||
S::NEEDS_LATEST_FRAGMENT
|
||||
}
|
||||
|
||||
fn render_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Value>,
|
||||
retained: Option<&str>,
|
||||
) -> SectionTransition {
|
||||
let typed_snapshot;
|
||||
let previous = match previous {
|
||||
PreviousSectionState::Known(previous) => {
|
||||
@@ -236,7 +252,7 @@ impl<S: WorldStateSection> ErasedWorldStateSection for S {
|
||||
PreviousSectionState::Absent => PreviousSectionState::Absent,
|
||||
PreviousSectionState::Unknown => PreviousSectionState::Unknown,
|
||||
};
|
||||
let (snapshot, updates) = WorldStateSection::render_diff(self, previous);
|
||||
let (snapshot, updates) = self.render_retained_diff(previous, retained);
|
||||
(
|
||||
snapshot.map(|snapshot| section_snapshot(self, snapshot).unwrap_or(Value::Null)),
|
||||
updates,
|
||||
@@ -263,7 +279,11 @@ impl ErasedWorldStateSection for ExtensionWorldStateSection {
|
||||
self.0.matches_retained_fragment(role, text)
|
||||
}
|
||||
|
||||
fn render_diff(&self, previous: PreviousSectionState<'_, Value>) -> SectionTransition {
|
||||
fn render_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Value>,
|
||||
_retained: Option<&str>,
|
||||
) -> SectionTransition {
|
||||
let previous = match previous {
|
||||
PreviousSectionState::Absent => PreviousWorldStateSection::Absent,
|
||||
PreviousSectionState::Unknown => PreviousWorldStateSection::Unknown,
|
||||
@@ -350,6 +370,17 @@ pub(crate) trait WorldStateSection: Send + Sync + 'static {
|
||||
const ID: &'static str;
|
||||
type Snapshot: DeserializeOwned + Serialize;
|
||||
|
||||
/// Whether history recovery needs the most recent match instead of any match.
|
||||
const NEEDS_LATEST_FRAGMENT: bool = false;
|
||||
|
||||
fn render_retained_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Self::Snapshot>,
|
||||
_retained: Option<&str>,
|
||||
) -> SectionTransition<Self::Snapshot> {
|
||||
self.render_diff(previous)
|
||||
}
|
||||
|
||||
/// Whether the section contributes comparison state to persisted rollouts.
|
||||
fn should_persist(&self) -> bool {
|
||||
true
|
||||
@@ -620,7 +651,7 @@ impl WorldState {
|
||||
|
||||
pub(crate) fn render_full(&self) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
|
||||
self.render_with(RenderScope::FullContext, /*stored*/ None, |_, _| {
|
||||
PreviousSectionState::Absent
|
||||
(PreviousSectionState::Absent, None)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -628,22 +659,48 @@ impl WorldState {
|
||||
pub(crate) fn render_history_diff<'a>(
|
||||
&self,
|
||||
previous: Option<&WorldStateSnapshot>,
|
||||
items: impl IntoIterator<Item = &'a ResponseItem> + Clone,
|
||||
items: impl IntoIterator<Item = &'a ResponseItem, IntoIter: DoubleEndedIterator> + Clone,
|
||||
) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
|
||||
self.render_with(RenderScope::HistoryDiff, previous, |id, section| {
|
||||
if let Some(previous) = previous.and_then(|previous| previous.section(id)) {
|
||||
if section.has_retained_fragment_matcher()
|
||||
&& !has_retained_fragment(items.clone(), section)
|
||||
{
|
||||
PreviousSectionState::Absent
|
||||
} else {
|
||||
PreviousSectionState::Known(previous)
|
||||
}
|
||||
} else if has_legacy_fragment(items.clone(), section) {
|
||||
PreviousSectionState::Unknown
|
||||
} else {
|
||||
PreviousSectionState::Absent
|
||||
let stored = previous.and_then(|previous| previous.section(id));
|
||||
if let Some(stored) = stored
|
||||
&& !section.has_retained_fragment_matcher()
|
||||
{
|
||||
return (PreviousSectionState::Known(stored), None);
|
||||
}
|
||||
let matches = |role: &str, text: &str| {
|
||||
if stored.is_some() {
|
||||
section.matches_retained_fragment(role, text)
|
||||
} else {
|
||||
section.matches_legacy_fragment(role, text)
|
||||
}
|
||||
};
|
||||
let mut fragments = items
|
||||
.clone()
|
||||
.into_iter()
|
||||
.filter_map(|item| match item {
|
||||
ResponseItem::Message { role, content, .. } => Some((role, content)),
|
||||
_ => None,
|
||||
})
|
||||
.flat_map(|(role, content)| {
|
||||
content.iter().filter_map(move |item| match item {
|
||||
ContentItem::InputText { text } if matches(role, text) => {
|
||||
Some(text.as_str())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
});
|
||||
let retained = if section.needs_latest_fragment() {
|
||||
fragments.next_back()
|
||||
} else {
|
||||
fragments.next()
|
||||
};
|
||||
let state = match (stored, retained) {
|
||||
(_, None) => PreviousSectionState::Absent,
|
||||
(None, Some(_)) => PreviousSectionState::Unknown,
|
||||
(Some(stored), Some(_)) => PreviousSectionState::Known(stored),
|
||||
};
|
||||
(state, retained)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -651,7 +708,10 @@ impl WorldState {
|
||||
&self,
|
||||
scope: RenderScope,
|
||||
stored: Option<&WorldStateSnapshot>,
|
||||
mut previous: impl FnMut(&str, &dyn ErasedWorldStateSection) -> PreviousSectionState<'a, Value>,
|
||||
mut previous: impl FnMut(
|
||||
&str,
|
||||
&dyn ErasedWorldStateSection,
|
||||
) -> (PreviousSectionState<'a, Value>, Option<&'a str>),
|
||||
) -> (WorldStateSnapshot, Vec<WorldStateUpdate>) {
|
||||
let mut snapshot = WorldStateSnapshot::default();
|
||||
let mut updates = Vec::new();
|
||||
@@ -659,8 +719,8 @@ impl WorldState {
|
||||
if matches!(scope, RenderScope::FullContext) && !section.include_in_full_context() {
|
||||
continue;
|
||||
}
|
||||
let prior = previous(id, section.as_ref());
|
||||
let (section_snapshot, section_updates) = section.render_diff(prior);
|
||||
let (prior, retained) = previous(id, section.as_ref());
|
||||
let (section_snapshot, section_updates) = section.render_diff(prior, retained);
|
||||
// A skipped snapshot retains stored state even if history required a fresh render.
|
||||
let section_snapshot = section_snapshot
|
||||
.or_else(|| stored.and_then(|snapshot| snapshot.section(id)).cloned());
|
||||
@@ -675,44 +735,6 @@ impl WorldState {
|
||||
}
|
||||
}
|
||||
|
||||
fn has_retained_fragment<'a>(
|
||||
items: impl IntoIterator<Item = &'a ResponseItem>,
|
||||
section: &dyn ErasedWorldStateSection,
|
||||
) -> bool {
|
||||
items.into_iter().any(|item| {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::Message { role, content, .. }
|
||||
if content.iter().any(|content| {
|
||||
matches!(
|
||||
content,
|
||||
ContentItem::InputText { text }
|
||||
if section.matches_retained_fragment(role, text)
|
||||
)
|
||||
})
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn has_legacy_fragment<'a>(
|
||||
items: impl IntoIterator<Item = &'a ResponseItem>,
|
||||
section: &dyn ErasedWorldStateSection,
|
||||
) -> bool {
|
||||
items.into_iter().any(|item| {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::Message { role, content, .. }
|
||||
if content.iter().any(|content| {
|
||||
matches!(
|
||||
content,
|
||||
ContentItem::InputText { text }
|
||||
if section.matches_legacy_fragment(role, text)
|
||||
)
|
||||
})
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn remove_null_object_fields(value: &mut Value) {
|
||||
// RFC 7386 reserves object-valued nulls for deletion, but arrays are replaced whole.
|
||||
match value {
|
||||
|
||||
@@ -81,7 +81,7 @@ fn render_diff<S: WorldStateSection>(
|
||||
PreviousSectionState::Known(&previous_snapshot)
|
||||
}
|
||||
};
|
||||
ErasedWorldStateSection::render_diff(*after, previous).1
|
||||
ErasedWorldStateSection::render_diff(*after, previous, /*retained*/ None).1
|
||||
}
|
||||
|
||||
fn render_snapshot<S: WorldStateSection>(section: &S) -> String {
|
||||
@@ -109,9 +109,13 @@ fn sort_json(value: serde_json::Value) -> serde_json::Value {
|
||||
}
|
||||
|
||||
fn snapshot_value<S: WorldStateSection>(section: &S) -> serde_json::Value {
|
||||
ErasedWorldStateSection::render_diff(section, PreviousSectionState::Absent)
|
||||
.0
|
||||
.expect("world-state section snapshot should serialize to a non-null value")
|
||||
ErasedWorldStateSection::render_diff(
|
||||
section,
|
||||
PreviousSectionState::Absent,
|
||||
/*retained*/ None,
|
||||
)
|
||||
.0
|
||||
.expect("world-state section snapshot should serialize to a non-null value")
|
||||
}
|
||||
|
||||
/// Keeps text-only section assertions explicit when a section gains item output.
|
||||
@@ -165,7 +169,7 @@ impl WorldState {
|
||||
pub(crate) fn render_history_fragment_diff<'a>(
|
||||
&self,
|
||||
previous: Option<&WorldStateSnapshot>,
|
||||
items: impl IntoIterator<Item = &'a ResponseItem> + Clone,
|
||||
items: impl IntoIterator<Item = &'a ResponseItem, IntoIter: DoubleEndedIterator> + Clone,
|
||||
) -> (WorldStateSnapshot, Vec<Box<dyn ContextualUserFragment>>) {
|
||||
let (snapshot, updates) = self.render_history_diff(previous, items);
|
||||
(snapshot, expect_fragments(updates))
|
||||
|
||||
Reference in New Issue
Block a user