Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 50 additions & 16 deletions codex-rs/core/src/context/world_state/collaboration_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,36 +3,66 @@ use super::WorldStateSection;
use crate::context::ContextualUserFragment;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
use codex_protocol::openai_models::CollaborationModeMessages;
use codex_protocol::protocol::COLLABORATION_MODE_CLOSE_TAG;
use codex_protocol::protocol::COLLABORATION_MODE_OPEN_TAG;
use serde::Deserialize;
use serde::Serialize;

/// Collaboration-mode instructions currently visible to the model.
#[derive(Clone, Debug)]
pub(crate) struct CollaborationModeState {
mode: ModeKind,
instructions: String,
model: String,
instructions: Option<String>,
}

impl CollaborationModeState {
pub(crate) fn from_collaboration_mode(collaboration_mode: &CollaborationMode) -> Option<Self> {
collaboration_mode
.settings
.developer_instructions
.clone()
.filter(|instructions| !instructions.is_empty())
.map(|instructions| Self {
mode: collaboration_mode.mode,
instructions,
})
pub(crate) fn from_collaboration_mode(
collaboration_mode: &CollaborationMode,
catalog_messages: Option<&CollaborationModeMessages>,
) -> Self {
let catalog_instructions =
catalog_messages.and_then(|messages| match collaboration_mode.mode {
ModeKind::Default => messages.default.as_ref(),
ModeKind::Plan => messages.plan.as_ref(),
ModeKind::PairProgramming | ModeKind::Execute => None,
});

Self {
mode: collaboration_mode.mode,
model: collaboration_mode.settings.model.clone(),
instructions: catalog_instructions.cloned().or_else(|| {
collaboration_mode
.settings
.developer_instructions
.clone()
.filter(|instructions| !instructions.is_empty())
}),
}
}
}

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(untagged)]
pub(crate) enum CollaborationModeSnapshot {
Current { mode: ModeKind, model: String },
Legacy(ModeKind),
}

impl WorldStateSection for CollaborationModeState {
const ID: &'static str = "collaboration_mode";
type Snapshot = ModeKind;
type Snapshot = CollaborationModeSnapshot;

fn snapshot(&self) -> Self::Snapshot {
self.mode
CollaborationModeSnapshot::Current {
mode: self.mode,
model: self.model.clone(),
}
}

fn should_persist(&self) -> bool {
self.instructions.is_some()
}

fn matches_legacy_fragment(role: &str, text: &str) -> bool {
Expand All @@ -51,14 +81,18 @@ impl WorldStateSection for CollaborationModeState {
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> Option<Box<dyn ContextualUserFragment>> {
if matches!(previous, PreviousSectionState::Known(previous) if previous == &self.mode)
|| matches!(previous, PreviousSectionState::Unknown)
if matches!(
previous,
PreviousSectionState::Known(CollaborationModeSnapshot::Current { mode, model })
if *mode == self.mode && model == &self.model
) || matches!(previous, PreviousSectionState::Unknown)
|| (self.instructions.is_none() && matches!(previous, PreviousSectionState::Absent))
{
return None;
}

Some(Box::new(CollaborationModeInstructions {
instructions: self.instructions.clone(),
instructions: self.instructions.clone().unwrap_or_default(),
}))
}
}
Expand Down
107 changes: 101 additions & 6 deletions codex-rs/core/src/context/world_state/collaboration_mode_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use crate::context::world_state::WorldState;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Settings;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::CollaborationModeMessages;
use pretty_assertions::assert_eq;

#[test]
fn snapshots() {
Expand All @@ -31,7 +33,7 @@ fn snapshots() {
fn persisted_instructions_are_restored_only_when_missing_from_history() {
let state = collaboration_mode_state(ModeKind::Default, "pair with the user");
let retained: ResponseItem = ContextualUserFragment::into(CollaborationModeInstructions {
instructions: state.instructions.clone(),
instructions: state.instructions.clone().expect("test instructions"),
});
let mut world_state = WorldState::default();
world_state.add_section(state);
Expand All @@ -53,14 +55,107 @@ fn persisted_instructions_are_restored_only_when_missing_from_history() {
);
}

fn collaboration_mode_state(mode: ModeKind, instructions: &str) -> CollaborationModeState {
CollaborationModeState::from_collaboration_mode(&CollaborationMode {
#[test]
fn catalog_collaboration_messages_select_mode_variant() {
let messages = CollaborationModeMessages {
default: Some("catalog default instructions".to_string()),
plan: Some("catalog plan instructions".to_string()),
};

for (mode, expected) in [
(ModeKind::Default, "catalog default instructions"),
(ModeKind::Plan, "catalog plan instructions"),
] {
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(mode, Some("legacy instructions")),
Some(&messages),
);

assert_eq!(state.instructions.as_deref(), Some(expected));
}
}

#[test]
fn empty_catalog_collaboration_message_suppresses_legacy_instructions() {
let messages = CollaborationModeMessages {
default: None,
plan: Some(String::new()),
};
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")),
Some(&messages),
);

assert_eq!(
state
.render_diff(PreviousSectionState::Absent)
.expect("explicit empty collaboration message")
.render(),
format!("{COLLABORATION_MODE_OPEN_TAG}{COLLABORATION_MODE_CLOSE_TAG}")
);
}

#[test]
fn missing_catalog_collaboration_message_uses_legacy_instructions() {
let messages = CollaborationModeMessages {
default: Some("catalog default instructions".to_string()),
plan: None,
};
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")),
Some(&messages),
);

assert_eq!(
state.instructions.as_deref(),
Some("legacy plan instructions")
);
}

#[test]
fn legacy_collaboration_mode_snapshots_refresh_catalog_messages_once() {
let previous = serde_json::from_str::<CollaborationModeSnapshot>("\"default\"")
.expect("legacy collaboration mode snapshot");

for instructions in ["catalog instructions", ""] {
let messages = CollaborationModeMessages {
default: Some(instructions.to_string()),
plan: None,
};
let state = CollaborationModeState::from_collaboration_mode(
&collaboration_mode(ModeKind::Default, Some("stale legacy instructions")),
Some(&messages),
);

assert_eq!(
state
.render_diff(PreviousSectionState::Known(&previous))
.expect("legacy snapshot should refresh collaboration instructions")
.render(),
format!("{COLLABORATION_MODE_OPEN_TAG}{instructions}{COLLABORATION_MODE_CLOSE_TAG}")
);
assert!(
state
.render_diff(PreviousSectionState::Known(&state.snapshot()))
.is_none()
);
}
}

fn collaboration_mode(mode: ModeKind, instructions: Option<&str>) -> CollaborationMode {
CollaborationMode {
mode,
settings: Settings {
model: "test-model".to_string(),
reasoning_effort: None,
developer_instructions: Some(instructions.to_string()),
developer_instructions: instructions.map(str::to_string),
},
})
.expect("test collaboration mode should have instructions")
}
}

fn collaboration_mode_state(mode: ModeKind, instructions: &str) -> CollaborationModeState {
CollaborationModeState::from_collaboration_mode(
&collaboration_mode(mode, Some(instructions)),
/*catalog_messages*/ None,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ expression: "render_section_cases(&[(Absent, Absent), (Absent, Known(&default)),
Absent -> Absent
None

Absent -> "default" (role - developer)
Absent -> {"mode":"default","model":"test-model"} (role - developer)
<collaboration_mode>pair with the user</collaboration_mode>

"default" -> "default"
{"mode":"default","model":"test-model"} -> {"mode":"default","model":"test-model"}
None

"default" -> "default"
{"mode":"default","model":"test-model"} -> {"mode":"default","model":"test-model"}
None

"default" -> "plan" (role - developer)
{"mode":"default","model":"test-model"} -> {"mode":"plan","model":"test-model"} (role - developer)
<collaboration_mode>make a plan</collaboration_mode>

Unknown -> "default"
Unknown -> {"mode":"default","model":"test-model"}
None
3 changes: 3 additions & 0 deletions codex-rs/core/src/guardian/review_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,7 @@ mod tests {
instructions_template: None,
instructions_variables: None,
approvals: None,
collaboration_modes: None,
auto_review: Some(AutoReviewMessages {
policy: Some("Use the catalog Guardian policy.".to_string()),
policy_template: Some(catalog_template.to_string()),
Expand Down Expand Up @@ -1486,6 +1487,7 @@ mod tests {
instructions_template: None,
instructions_variables: None,
approvals: None,
collaboration_modes: None,
auto_review: Some(AutoReviewMessages {
policy: Some(String::new()),
policy_template: None,
Expand Down Expand Up @@ -1527,6 +1529,7 @@ mod tests {
instructions_template: None,
instructions_variables: None,
approvals: None,
collaboration_modes: None,
auto_review: Some(AutoReviewMessages {
policy: Some(catalog_policy.to_string()),
policy_template: Some(String::new()),
Expand Down
14 changes: 9 additions & 5 deletions codex-rs/core/src/session/world_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,15 @@ impl Session {
.enabled(Feature::RequestPermissionsTool),
));
}
if turn_context.config.include_collaboration_mode_instructions
&& let Some(collaboration_mode) =
CollaborationModeState::from_collaboration_mode(&turn_context.collaboration_mode())
{
world_state.add_section(collaboration_mode);
if turn_context.config.include_collaboration_mode_instructions {
world_state.add_section(CollaborationModeState::from_collaboration_mode(
&turn_context.collaboration_mode(),
turn_context
.model_info
.model_messages
.as_ref()
.and_then(|messages| messages.collaboration_modes.as_ref()),
));
}
if turn_context.config.include_environment_context {
let current_date = self
Expand Down
23 changes: 22 additions & 1 deletion codex-rs/core/tests/common/test_codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionConfiguredEvent;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::ThreadHistoryMode;
use codex_protocol::protocol::ThreadSettingsOverrides;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::protocol::TurnEnvironmentSelections;
use codex_protocol::user_input::UserInput;
Expand All @@ -66,6 +67,7 @@ use crate::responses::output_value_to_text;
use crate::responses::start_mock_server;
use crate::streaming_sse::StreamingSseServer;
use crate::test_environment;
use crate::wait_for_event;
use crate::wait_for_event_match;
use crate::wait_for_event_with_timeout;
use wiremock::Match;
Expand Down Expand Up @@ -831,6 +833,25 @@ impl TestCodex {
.await
}

/// Submits a text turn without changing the current thread settings.
pub async fn submit_text_turn(&self, prompt: &str) -> Result<()> {
self.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: prompt.into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: ThreadSettingsOverrides::default(),
})
.await?;

wait_for_event(&self.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
Ok(())
}

pub async fn submit_turn_with_permission_profile(
&self,
prompt: &str,
Expand Down Expand Up @@ -960,7 +981,7 @@ impl TestCodex {
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
thread_settings: ThreadSettingsOverrides {
environments: turn_environment_selections,
approval_policy: Some(approval_policy),
sandbox_policy: Some(sandbox_policy),
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/tests/suite/catalog_permission_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ async fn catalog_permission_message_loaded_from_remote_models_is_sent() -> Resul
instructions_template: None,
instructions_variables: None,
approvals: None,
collaboration_modes: None,
auto_review: None,
permissions: Some(PermissionMessages {
danger_full_access: None,
Expand Down
Loading
Loading