mirror of
https://github.com/openai/codex.git
synced 2026-11-12 22:42:36 +00:00
fix(app-server) revert null instructions changes (#17047)
This commit is contained in:
1 parent
f480b98984
commit
6c36e7d688
48 files changed
+102
-671
No files matched your search
@@ -27,7 +27,7 @@ model?: string | null, modelProvider?: string | null, serviceTier?: ServiceTier
|
|||||||
* Override where approval requests are routed for review on this thread
|
* Override where approval requests are routed for review on this thread
|
||||||
* and subsequent turns.
|
* and subsequent turns.
|
||||||
*/
|
*/
|
||||||
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, ephemeral?: boolean, /**
|
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, ephemeral?: boolean, /**
|
||||||
* If true, persist additional rollout EventMsg variants required to
|
* If true, persist additional rollout EventMsg variants required to
|
||||||
* reconstruct a richer thread history on subsequent resume/fork/read.
|
* reconstruct a richer thread history on subsequent resume/fork/read.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ model?: string | null, modelProvider?: string | null, serviceTier?: ServiceTier
|
|||||||
* Override where approval requests are routed for review on this thread
|
* Override where approval requests are routed for review on this thread
|
||||||
* and subsequent turns.
|
* and subsequent turns.
|
||||||
*/
|
*/
|
||||||
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, personality?: Personality | null, /**
|
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, /**
|
||||||
* If true, persist additional rollout EventMsg variants required to
|
* If true, persist additional rollout EventMsg variants required to
|
||||||
* reconstruct a richer thread history on subsequent resume/fork/read.
|
* reconstruct a richer thread history on subsequent resume/fork/read.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ export type ThreadStartParams = {model?: string | null, modelProvider?: string |
|
|||||||
* Override where approval requests are routed for review on this thread
|
* Override where approval requests are routed for review on this thread
|
||||||
* and subsequent turns.
|
* and subsequent turns.
|
||||||
*/
|
*/
|
||||||
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, serviceName?: string | null, baseInstructions?: string | null | null, developerInstructions?: string | null | null, personality?: Personality | null, ephemeral?: boolean | null, /**
|
approvalsReviewer?: ApprovalsReviewer | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, serviceName?: string | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, ephemeral?: boolean | null, /**
|
||||||
* If true, opt into emitting raw Responses API items on the event stream.
|
* If true, opt into emitting raw Responses API items on the event stream.
|
||||||
* This is for internal use only (e.g. Codex Cloud).
|
* This is for internal use only (e.g. Codex Cloud).
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -2603,22 +2603,10 @@ pub struct ThreadStartParams {
|
|||||||
pub config: Option<HashMap<String, JsonValue>>,
|
pub config: Option<HashMap<String, JsonValue>>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub service_name: Option<String>,
|
pub service_name: Option<String>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub base_instructions: Option<Option<String>>,
|
pub base_instructions: Option<String>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub developer_instructions: Option<Option<String>>,
|
pub developer_instructions: Option<String>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub personality: Option<Personality>,
|
pub personality: Option<Personality>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
@@ -2733,22 +2721,10 @@ pub struct ThreadResumeParams {
|
|||||||
pub sandbox: Option<SandboxMode>,
|
pub sandbox: Option<SandboxMode>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub config: Option<HashMap<String, serde_json::Value>>,
|
pub config: Option<HashMap<String, serde_json::Value>>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub base_instructions: Option<Option<String>>,
|
pub base_instructions: Option<String>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub developer_instructions: Option<Option<String>>,
|
pub developer_instructions: Option<String>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub personality: Option<Personality>,
|
pub personality: Option<Personality>,
|
||||||
/// If true, persist additional rollout EventMsg variants required to
|
/// If true, persist additional rollout EventMsg variants required to
|
||||||
@@ -2822,22 +2798,10 @@ pub struct ThreadForkParams {
|
|||||||
pub sandbox: Option<SandboxMode>,
|
pub sandbox: Option<SandboxMode>,
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub config: Option<HashMap<String, serde_json::Value>>,
|
pub config: Option<HashMap<String, serde_json::Value>>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub base_instructions: Option<Option<String>>,
|
pub base_instructions: Option<String>,
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "super::serde_helpers::deserialize_double_option",
|
|
||||||
serialize_with = "super::serde_helpers::serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
#[ts(optional = nullable)]
|
#[ts(optional = nullable)]
|
||||||
pub developer_instructions: Option<Option<String>>,
|
pub developer_instructions: Option<String>,
|
||||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||||
pub ephemeral: bool,
|
pub ephemeral: bool,
|
||||||
/// If true, persist additional rollout EventMsg variants required to
|
/// If true, persist additional rollout EventMsg variants required to
|
||||||
@@ -8364,35 +8328,6 @@ mod tests {
|
|||||||
assert_eq!(serialized_without_override.get("serviceTier"), None);
|
assert_eq!(serialized_without_override.get("serviceTier"), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn thread_start_params_preserve_explicit_null_instructions() {
|
|
||||||
let params: ThreadStartParams = serde_json::from_value(json!({
|
|
||||||
"baseInstructions": null,
|
|
||||||
"developerInstructions": null,
|
|
||||||
}))
|
|
||||||
.expect("params should deserialize");
|
|
||||||
assert_eq!(params.base_instructions, Some(None));
|
|
||||||
assert_eq!(params.developer_instructions, Some(None));
|
|
||||||
|
|
||||||
let serialized = serde_json::to_value(¶ms).expect("params should serialize");
|
|
||||||
assert_eq!(
|
|
||||||
serialized.get("baseInstructions"),
|
|
||||||
Some(&serde_json::Value::Null)
|
|
||||||
);
|
|
||||||
assert_eq!(
|
|
||||||
serialized.get("developerInstructions"),
|
|
||||||
Some(&serde_json::Value::Null)
|
|
||||||
);
|
|
||||||
|
|
||||||
let serialized_without_override =
|
|
||||||
serde_json::to_value(ThreadStartParams::default()).expect("params should serialize");
|
|
||||||
assert_eq!(serialized_without_override.get("baseInstructions"), None);
|
|
||||||
assert_eq!(
|
|
||||||
serialized_without_override.get("developerInstructions"),
|
|
||||||
None
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn turn_start_params_preserve_explicit_null_service_tier() {
|
fn turn_start_params_preserve_explicit_null_service_tier() {
|
||||||
let params: TurnStartParams = serde_json::from_value(json!({
|
let params: TurnStartParams = serde_json::from_value(json!({
|
||||||
|
|||||||
@@ -2473,8 +2473,8 @@ impl CodexMessageProcessor {
|
|||||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||||
sandbox: Option<SandboxMode>,
|
sandbox: Option<SandboxMode>,
|
||||||
base_instructions: Option<Option<String>>,
|
base_instructions: Option<String>,
|
||||||
developer_instructions: Option<Option<String>>,
|
developer_instructions: Option<String>,
|
||||||
personality: Option<Personality>,
|
personality: Option<Personality>,
|
||||||
) -> ConfigOverrides {
|
) -> ConfigOverrides {
|
||||||
ConfigOverrides {
|
ConfigOverrides {
|
||||||
@@ -4363,13 +4363,6 @@ impl CodexMessageProcessor {
|
|||||||
developer_instructions,
|
developer_instructions,
|
||||||
/*personality*/ None,
|
/*personality*/ None,
|
||||||
);
|
);
|
||||||
if typesafe_overrides.base_instructions.is_none()
|
|
||||||
&& let Ok(history) = RolloutRecorder::get_rollout_history(&rollout_path).await
|
|
||||||
&& let Some(base_instructions) = history.get_base_instructions()
|
|
||||||
{
|
|
||||||
typesafe_overrides.base_instructions =
|
|
||||||
Some(base_instructions.map(|base_instructions| base_instructions.text));
|
|
||||||
}
|
|
||||||
typesafe_overrides.ephemeral = ephemeral.then_some(true);
|
typesafe_overrides.ephemeral = ephemeral.then_some(true);
|
||||||
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
||||||
let cloud_requirements = self.current_cloud_requirements();
|
let cloud_requirements = self.current_cloud_requirements();
|
||||||
|
|||||||
@@ -84,7 +84,6 @@ pub fn create_fake_rollout_with_source(
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: model_provider.map(str::to_string),
|
model_provider: model_provider.map(str::to_string),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
@@ -168,7 +167,6 @@ pub fn create_fake_rollout_with_text_elements(
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: model_provider.map(str::to_string),
|
model_provider: model_provider.map(str::to_string),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -26,8 +26,6 @@ use codex_app_server_protocol::TurnStatus;
|
|||||||
use codex_app_server_protocol::UserInput;
|
use codex_app_server_protocol::UserInput;
|
||||||
use codex_config::types::AuthCredentialsStoreMode;
|
use codex_config::types::AuthCredentialsStoreMode;
|
||||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||||
use core_test_support::responses;
|
|
||||||
use core_test_support::skip_if_no_network;
|
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
@@ -185,170 +183,6 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn thread_fork_honors_explicit_null_thread_instructions() -> Result<()> {
|
|
||||||
skip_if_no_network!(Ok(()));
|
|
||||||
|
|
||||||
let server = responses::start_mock_server().await;
|
|
||||||
let body = responses::sse(vec![
|
|
||||||
responses::ev_response_created("resp-1"),
|
|
||||||
responses::ev_assistant_message("msg-1", "Done"),
|
|
||||||
responses::ev_completed("resp-1"),
|
|
||||||
]);
|
|
||||||
let response_mock =
|
|
||||||
responses::mount_sse_sequence(&server, vec![body.clone(), body.clone(), body]).await;
|
|
||||||
|
|
||||||
let codex_home = TempDir::new()?;
|
|
||||||
create_config_toml(codex_home.path(), &server.uri())?;
|
|
||||||
let config_path = codex_home.path().join("config.toml");
|
|
||||||
let mut config_toml = std::fs::read_to_string(&config_path)?;
|
|
||||||
let first_table_index = config_toml
|
|
||||||
.find("\n[")
|
|
||||||
.expect("test config must include a table header");
|
|
||||||
config_toml.insert_str(
|
|
||||||
first_table_index,
|
|
||||||
"\ndeveloper_instructions = \"Config developer instructions sentinel\"\n",
|
|
||||||
);
|
|
||||||
std::fs::write(config_path, config_toml)?;
|
|
||||||
|
|
||||||
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 disabled_instruction_config = json!({
|
|
||||||
"include_permissions_instructions": false,
|
|
||||||
"include_apps_instructions": false,
|
|
||||||
"include_environment_context": false,
|
|
||||||
"features.apps": false,
|
|
||||||
"features.plugins": false,
|
|
||||||
"features.codex_hooks": false,
|
|
||||||
"skills.bundled.enabled": false,
|
|
||||||
});
|
|
||||||
|
|
||||||
let fork_params = [
|
|
||||||
(
|
|
||||||
json!({
|
|
||||||
"threadId": conversation_id.clone(),
|
|
||||||
"config": disabled_instruction_config.clone(),
|
|
||||||
}),
|
|
||||||
/*expect_instructions*/ true,
|
|
||||||
),
|
|
||||||
(
|
|
||||||
json!({
|
|
||||||
"threadId": conversation_id.clone(),
|
|
||||||
"config": disabled_instruction_config.clone(),
|
|
||||||
"baseInstructions": null,
|
|
||||||
"developerInstructions": null,
|
|
||||||
}),
|
|
||||||
/*expect_instructions*/ false,
|
|
||||||
),
|
|
||||||
];
|
|
||||||
|
|
||||||
let mut forked_thread_ids = Vec::new();
|
|
||||||
for (params, _expect_instructions) in fork_params {
|
|
||||||
let fork_id = mcp.send_raw_request("thread/fork", Some(params)).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)?;
|
|
||||||
forked_thread_ids.push(thread.id.clone());
|
|
||||||
|
|
||||||
let turn_id = mcp
|
|
||||||
.send_turn_start_request(TurnStartParams {
|
|
||||||
thread_id: thread.id,
|
|
||||||
input: vec![UserInput::Text {
|
|
||||||
text: "continue".to_string(),
|
|
||||||
text_elements: Vec::new(),
|
|
||||||
}],
|
|
||||||
..Default::default()
|
|
||||||
})
|
|
||||||
.await?;
|
|
||||||
let turn_resp: JSONRPCResponse = timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_id)),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
let _: TurnStartResponse = to_response::<TurnStartResponse>(turn_resp)?;
|
|
||||||
timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_notification_message("turn/completed"),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
}
|
|
||||||
|
|
||||||
let refork_id = mcp
|
|
||||||
.send_raw_request(
|
|
||||||
"thread/fork",
|
|
||||||
Some(json!({
|
|
||||||
"threadId": forked_thread_ids[1].clone(),
|
|
||||||
"config": disabled_instruction_config.clone(),
|
|
||||||
})),
|
|
||||||
)
|
|
||||||
.await?;
|
|
||||||
let refork_resp: JSONRPCResponse = timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_response_message(RequestId::Integer(refork_id)),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
let ThreadForkResponse { thread, .. } = to_response::<ThreadForkResponse>(refork_resp)?;
|
|
||||||
let turn_id = mcp
|
|
||||||
.send_turn_start_request(TurnStartParams {
|
|
||||||
thread_id: thread.id,
|
|
||||||
input: vec![UserInput::Text {
|
|
||||||
text: "continue again".to_string(),
|
|
||||||
text_elements: Vec::new(),
|
|
||||||
}],
|
|
||||||
..Default::default()
|
|
||||||
})
|
|
||||||
.await?;
|
|
||||||
let turn_resp: JSONRPCResponse = timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_id)),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
let _: TurnStartResponse = to_response::<TurnStartResponse>(turn_resp)?;
|
|
||||||
timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_notification_message("turn/completed"),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
|
|
||||||
let requests = response_mock.requests();
|
|
||||||
assert_eq!(requests.len(), 3);
|
|
||||||
for (request, expect_instructions) in requests.into_iter().zip([true, false, false]) {
|
|
||||||
let payload = request.body_json();
|
|
||||||
assert_eq!(
|
|
||||||
payload.get("instructions").is_some(),
|
|
||||||
expect_instructions,
|
|
||||||
"unexpected instructions field in payload: {payload:?}"
|
|
||||||
);
|
|
||||||
let developer_texts = request.message_input_texts("developer");
|
|
||||||
assert_eq!(
|
|
||||||
developer_texts
|
|
||||||
.iter()
|
|
||||||
.any(|text| { text.contains("Config developer instructions sentinel") }),
|
|
||||||
expect_instructions,
|
|
||||||
"unexpected config developer instruction presence: {developer_texts:?}"
|
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
developer_texts.iter().all(|text| !text.is_empty()),
|
|
||||||
"did not expect empty developer instruction messages: {developer_texts:?}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn thread_fork_tracks_thread_initialized_analytics() -> Result<()> {
|
async fn thread_fork_tracks_thread_initialized_analytics() -> Result<()> {
|
||||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||||
|
|||||||
@@ -377,7 +377,6 @@ stream_max_retries = 0
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("mock_provider".to_string()),
|
model_provider: Some("mock_provider".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -152,120 +152,6 @@ async fn turn_start_sends_originator_header() -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn turn_start_honors_explicit_null_thread_instructions() -> Result<()> {
|
|
||||||
skip_if_no_network!(Ok(()));
|
|
||||||
|
|
||||||
let server = responses::start_mock_server().await;
|
|
||||||
let body = responses::sse(vec![
|
|
||||||
responses::ev_response_created("resp-1"),
|
|
||||||
responses::ev_assistant_message("msg-1", "Done"),
|
|
||||||
responses::ev_completed("resp-1"),
|
|
||||||
]);
|
|
||||||
let response_mock = responses::mount_sse_sequence(&server, vec![body.clone(), body]).await;
|
|
||||||
|
|
||||||
let codex_home = TempDir::new()?;
|
|
||||||
create_config_toml(codex_home.path(), &server.uri(), "never", &BTreeMap::new())?;
|
|
||||||
let config_path = codex_home.path().join("config.toml");
|
|
||||||
let mut config_toml = std::fs::read_to_string(&config_path)?;
|
|
||||||
let first_table_index = config_toml
|
|
||||||
.find("\n[")
|
|
||||||
.expect("test config must include a table header");
|
|
||||||
config_toml.insert_str(
|
|
||||||
first_table_index,
|
|
||||||
"\ndeveloper_instructions = \"Config developer instructions sentinel\"\n",
|
|
||||||
);
|
|
||||||
std::fs::write(config_path, config_toml)?;
|
|
||||||
|
|
||||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
||||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
|
||||||
|
|
||||||
let disabled_instruction_config = json!({
|
|
||||||
"include_permissions_instructions": false,
|
|
||||||
"include_apps_instructions": false,
|
|
||||||
"include_environment_context": false,
|
|
||||||
"features.apps": false,
|
|
||||||
"features.plugins": false,
|
|
||||||
"features.codex_hooks": false,
|
|
||||||
"skills.bundled.enabled": false,
|
|
||||||
});
|
|
||||||
|
|
||||||
let thread_start_params = [
|
|
||||||
(
|
|
||||||
json!({
|
|
||||||
"model": "mock-model",
|
|
||||||
"config": disabled_instruction_config.clone(),
|
|
||||||
}),
|
|
||||||
/*expect_instructions*/ true,
|
|
||||||
),
|
|
||||||
(
|
|
||||||
json!({
|
|
||||||
"model": "mock-model",
|
|
||||||
"config": disabled_instruction_config.clone(),
|
|
||||||
"baseInstructions": null,
|
|
||||||
"developerInstructions": null,
|
|
||||||
}),
|
|
||||||
/*expect_instructions*/ false,
|
|
||||||
),
|
|
||||||
];
|
|
||||||
|
|
||||||
for (params, _expect_instructions) in thread_start_params {
|
|
||||||
let thread_req = mcp.send_raw_request("thread/start", Some(params)).await?;
|
|
||||||
let thread_resp: JSONRPCResponse = timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
|
|
||||||
|
|
||||||
let turn_req = mcp
|
|
||||||
.send_turn_start_request(TurnStartParams {
|
|
||||||
thread_id: thread.id,
|
|
||||||
input: vec![V2UserInput::Text {
|
|
||||||
text: "Hello".to_string(),
|
|
||||||
text_elements: Vec::new(),
|
|
||||||
}],
|
|
||||||
..Default::default()
|
|
||||||
})
|
|
||||||
.await?;
|
|
||||||
timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
timeout(
|
|
||||||
DEFAULT_READ_TIMEOUT,
|
|
||||||
mcp.read_stream_until_notification_message("turn/completed"),
|
|
||||||
)
|
|
||||||
.await??;
|
|
||||||
}
|
|
||||||
|
|
||||||
let requests = response_mock.requests();
|
|
||||||
assert_eq!(requests.len(), 2);
|
|
||||||
for (request, expect_instructions) in requests.into_iter().zip([true, false]) {
|
|
||||||
let payload = request.body_json();
|
|
||||||
assert_eq!(
|
|
||||||
payload.get("instructions").is_some(),
|
|
||||||
expect_instructions,
|
|
||||||
"unexpected instructions field in payload: {payload:?}"
|
|
||||||
);
|
|
||||||
let developer_texts = request.message_input_texts("developer");
|
|
||||||
assert_eq!(
|
|
||||||
developer_texts
|
|
||||||
.iter()
|
|
||||||
.any(|text| { text.contains("Config developer instructions sentinel") }),
|
|
||||||
expect_instructions,
|
|
||||||
"unexpected config developer instruction presence: {developer_texts:?}"
|
|
||||||
);
|
|
||||||
assert!(
|
|
||||||
developer_texts.iter().all(|text| !text.is_empty()),
|
|
||||||
"did not expect empty developer instruction messages: {developer_texts:?}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> {
|
async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> {
|
||||||
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
||||||
|
|||||||
@@ -153,8 +153,7 @@ impl From<VerbosityConfig> for OpenAiVerbosity {
|
|||||||
#[derive(Debug, Serialize, Clone, PartialEq)]
|
#[derive(Debug, Serialize, Clone, PartialEq)]
|
||||||
pub struct ResponsesApiRequest {
|
pub struct ResponsesApiRequest {
|
||||||
pub model: String,
|
pub model: String,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
pub instructions: String,
|
||||||
pub instructions: Option<String>,
|
|
||||||
pub input: Vec<ResponseItem>,
|
pub input: Vec<ResponseItem>,
|
||||||
pub tools: Vec<serde_json::Value>,
|
pub tools: Vec<serde_json::Value>,
|
||||||
pub tool_choice: String,
|
pub tool_choice: String,
|
||||||
@@ -199,8 +198,7 @@ impl From<&ResponsesApiRequest> for ResponseCreateWsRequest {
|
|||||||
#[derive(Debug, Serialize)]
|
#[derive(Debug, Serialize)]
|
||||||
pub struct ResponseCreateWsRequest {
|
pub struct ResponseCreateWsRequest {
|
||||||
pub model: String,
|
pub model: String,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
pub instructions: String,
|
||||||
pub instructions: Option<String>,
|
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub previous_response_id: Option<String>,
|
pub previous_response_id: Option<String>,
|
||||||
pub input: Vec<ResponseItem>,
|
pub input: Vec<ResponseItem>,
|
||||||
|
|||||||
@@ -266,7 +266,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> {
|
|||||||
|
|
||||||
let request = ResponsesApiRequest {
|
let request = ResponsesApiRequest {
|
||||||
model: "gpt-test".into(),
|
model: "gpt-test".into(),
|
||||||
instructions: Some("Say hi".into()),
|
instructions: "Say hi".into(),
|
||||||
input: Vec::new(),
|
input: Vec::new(),
|
||||||
tools: Vec::new(),
|
tools: Vec::new(),
|
||||||
tool_choice: "auto".into(),
|
tool_choice: "auto".into(),
|
||||||
@@ -303,7 +303,7 @@ async fn azure_default_store_attaches_ids_and_headers() -> Result<()> {
|
|||||||
|
|
||||||
let request = ResponsesApiRequest {
|
let request = ResponsesApiRequest {
|
||||||
model: "gpt-test".into(),
|
model: "gpt-test".into(),
|
||||||
instructions: Some("Say hi".into()),
|
instructions: "Say hi".into(),
|
||||||
input: vec![ResponseItem::Message {
|
input: vec![ResponseItem::Message {
|
||||||
id: Some("msg_1".into()),
|
id: Some("msg_1".into()),
|
||||||
role: "user".into(),
|
role: "user".into(),
|
||||||
|
|||||||
@@ -403,11 +403,7 @@ impl ModelClient {
|
|||||||
ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth)
|
ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth)
|
||||||
.with_telemetry(Some(request_telemetry));
|
.with_telemetry(Some(request_telemetry));
|
||||||
|
|
||||||
let instructions = prompt
|
let instructions = prompt.base_instructions.text.clone();
|
||||||
.base_instructions
|
|
||||||
.as_ref()
|
|
||||||
.map(|base_instructions| base_instructions.text.clone())
|
|
||||||
.unwrap_or_default();
|
|
||||||
let input = prompt.get_formatted_input();
|
let input = prompt.get_formatted_input();
|
||||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||||
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
||||||
@@ -771,10 +767,7 @@ impl ModelClientSession {
|
|||||||
summary: ReasoningSummaryConfig,
|
summary: ReasoningSummaryConfig,
|
||||||
service_tier: Option<ServiceTier>,
|
service_tier: Option<ServiceTier>,
|
||||||
) -> Result<ResponsesApiRequest> {
|
) -> Result<ResponsesApiRequest> {
|
||||||
let instructions = prompt
|
let instructions = &prompt.base_instructions.text;
|
||||||
.base_instructions
|
|
||||||
.as_ref()
|
|
||||||
.map(|base_instructions| base_instructions.text.clone());
|
|
||||||
let input = prompt.get_formatted_input();
|
let input = prompt.get_formatted_input();
|
||||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||||
let default_reasoning_effort = model_info.default_reasoning_level;
|
let default_reasoning_effort = model_info.default_reasoning_level;
|
||||||
@@ -813,7 +806,7 @@ impl ModelClientSession {
|
|||||||
let prompt_cache_key = Some(self.client.state.conversation_id.to_string());
|
let prompt_cache_key = Some(self.client.state.conversation_id.to_string());
|
||||||
let request = ResponsesApiRequest {
|
let request = ResponsesApiRequest {
|
||||||
model: model_info.slug.clone(),
|
model: model_info.slug.clone(),
|
||||||
instructions,
|
instructions: instructions.clone(),
|
||||||
input,
|
input,
|
||||||
tools,
|
tools,
|
||||||
tool_choice: "auto".to_string(),
|
tool_choice: "auto".to_string(),
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ pub const REVIEW_EXIT_INTERRUPTED_TMPL: &str =
|
|||||||
include_str!("../templates/review/exit_interrupted.xml");
|
include_str!("../templates/review/exit_interrupted.xml");
|
||||||
|
|
||||||
/// API request payload for a single model turn
|
/// API request payload for a single model turn
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Default, Debug, Clone)]
|
||||||
pub struct Prompt {
|
pub struct Prompt {
|
||||||
/// Conversation context input items.
|
/// Conversation context input items.
|
||||||
pub input: Vec<ResponseItem>,
|
pub input: Vec<ResponseItem>,
|
||||||
@@ -35,7 +35,7 @@ pub struct Prompt {
|
|||||||
/// Whether parallel tool calls are permitted for this prompt.
|
/// Whether parallel tool calls are permitted for this prompt.
|
||||||
pub(crate) parallel_tool_calls: bool,
|
pub(crate) parallel_tool_calls: bool,
|
||||||
|
|
||||||
pub base_instructions: Option<BaseInstructions>,
|
pub base_instructions: BaseInstructions,
|
||||||
|
|
||||||
/// Optionally specify the personality of the model.
|
/// Optionally specify the personality of the model.
|
||||||
pub personality: Option<Personality>,
|
pub personality: Option<Personality>,
|
||||||
@@ -44,19 +44,6 @@ pub struct Prompt {
|
|||||||
pub output_schema: Option<Value>,
|
pub output_schema: Option<Value>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for Prompt {
|
|
||||||
fn default() -> Self {
|
|
||||||
Self {
|
|
||||||
input: Vec::new(),
|
|
||||||
tools: Vec::new(),
|
|
||||||
parallel_tool_calls: false,
|
|
||||||
base_instructions: Some(BaseInstructions::default()),
|
|
||||||
personality: None,
|
|
||||||
output_schema: None,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Prompt {
|
impl Prompt {
|
||||||
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
|
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
|
||||||
let mut input = self.input.clone();
|
let mut input = self.input.clone();
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ fn serializes_text_verbosity_when_set() {
|
|||||||
let tools: Vec<serde_json::Value> = vec![];
|
let tools: Vec<serde_json::Value> = vec![];
|
||||||
let req = ResponsesApiRequest {
|
let req = ResponsesApiRequest {
|
||||||
model: "gpt-5.1".to_string(),
|
model: "gpt-5.1".to_string(),
|
||||||
instructions: Some("i".to_string()),
|
instructions: "i".to_string(),
|
||||||
input,
|
input,
|
||||||
tools,
|
tools,
|
||||||
tool_choice: "auto".to_string(),
|
tool_choice: "auto".to_string(),
|
||||||
@@ -58,7 +58,7 @@ fn serializes_text_schema_with_strict_format() {
|
|||||||
|
|
||||||
let req = ResponsesApiRequest {
|
let req = ResponsesApiRequest {
|
||||||
model: "gpt-5.1".to_string(),
|
model: "gpt-5.1".to_string(),
|
||||||
instructions: Some("i".to_string()),
|
instructions: "i".to_string(),
|
||||||
input,
|
input,
|
||||||
tools,
|
tools,
|
||||||
tool_choice: "auto".to_string(),
|
tool_choice: "auto".to_string(),
|
||||||
@@ -96,7 +96,7 @@ fn omits_text_when_not_set() {
|
|||||||
let tools: Vec<serde_json::Value> = vec![];
|
let tools: Vec<serde_json::Value> = vec![];
|
||||||
let req = ResponsesApiRequest {
|
let req = ResponsesApiRequest {
|
||||||
model: "gpt-5.1".to_string(),
|
model: "gpt-5.1".to_string(),
|
||||||
instructions: Some("i".to_string()),
|
instructions: "i".to_string(),
|
||||||
input,
|
input,
|
||||||
tools,
|
tools,
|
||||||
tool_choice: "auto".to_string(),
|
tool_choice: "auto".to_string(),
|
||||||
@@ -119,7 +119,7 @@ fn omits_text_when_not_set() {
|
|||||||
fn serializes_flex_service_tier_when_set() {
|
fn serializes_flex_service_tier_when_set() {
|
||||||
let req = ResponsesApiRequest {
|
let req = ResponsesApiRequest {
|
||||||
model: "gpt-5.1".to_string(),
|
model: "gpt-5.1".to_string(),
|
||||||
instructions: Some("i".to_string()),
|
instructions: "i".to_string(),
|
||||||
input: vec![],
|
input: vec![],
|
||||||
tools: vec![],
|
tools: vec![],
|
||||||
tool_choice: "auto".to_string(),
|
tool_choice: "auto".to_string(),
|
||||||
|
|||||||
+19
-53
@@ -579,15 +579,11 @@ impl Codex {
|
|||||||
let model_info = models_manager
|
let model_info = models_manager
|
||||||
.get_model_info(model.as_str(), &config.to_models_manager_config())
|
.get_model_info(model.as_str(), &config.to_models_manager_config())
|
||||||
.await;
|
.await;
|
||||||
let base_instructions = match config.base_instructions.clone() {
|
let base_instructions = config
|
||||||
Some(base_instructions) => base_instructions,
|
.base_instructions
|
||||||
None => conversation_history
|
.clone()
|
||||||
.get_base_instructions()
|
.or_else(|| conversation_history.get_base_instructions().map(|s| s.text))
|
||||||
.map(|base_instructions| {
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality));
|
||||||
base_instructions.map(|base_instructions| base_instructions.text)
|
|
||||||
})
|
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
|
||||||
};
|
|
||||||
|
|
||||||
// Respect thread-start tools. When missing (resumed/forked threads), read from the db
|
// Respect thread-start tools. When missing (resumed/forked threads), read from the db
|
||||||
// first, then fall back to rollout-file tools.
|
// first, then fall back to rollout-file tools.
|
||||||
@@ -616,14 +612,6 @@ impl Codex {
|
|||||||
dynamic_tools
|
dynamic_tools
|
||||||
};
|
};
|
||||||
|
|
||||||
let developer_instructions_override = config
|
|
||||||
.developer_instructions_override
|
|
||||||
.clone()
|
|
||||||
.or_else(|| conversation_history.get_developer_instructions());
|
|
||||||
let developer_instructions = developer_instructions_override
|
|
||||||
.clone()
|
|
||||||
.unwrap_or_else(|| config.developer_instructions.clone());
|
|
||||||
|
|
||||||
// TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode
|
// TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode
|
||||||
// to avoid extracting these fields separately and constructing CollaborationMode here.
|
// to avoid extracting these fields separately and constructing CollaborationMode here.
|
||||||
let collaboration_mode = CollaborationMode {
|
let collaboration_mode = CollaborationMode {
|
||||||
@@ -639,8 +627,7 @@ impl Codex {
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
service_tier: config.service_tier,
|
service_tier: config.service_tier,
|
||||||
developer_instructions,
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override,
|
|
||||||
user_instructions,
|
user_instructions,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions,
|
base_instructions,
|
||||||
@@ -1105,10 +1092,6 @@ pub(crate) struct SessionConfiguration {
|
|||||||
/// Developer instructions that supplement the base instructions.
|
/// Developer instructions that supplement the base instructions.
|
||||||
developer_instructions: Option<String>,
|
developer_instructions: Option<String>,
|
||||||
|
|
||||||
/// Explicit developer instructions override, preserving `null` as distinct
|
|
||||||
/// from a missing override.
|
|
||||||
developer_instructions_override: Option<Option<String>>,
|
|
||||||
|
|
||||||
/// Model instructions that are appended to the base instructions.
|
/// Model instructions that are appended to the base instructions.
|
||||||
user_instructions: Option<String>,
|
user_instructions: Option<String>,
|
||||||
|
|
||||||
@@ -1116,7 +1099,7 @@ pub(crate) struct SessionConfiguration {
|
|||||||
personality: Option<Personality>,
|
personality: Option<Personality>,
|
||||||
|
|
||||||
/// Base instructions for the session.
|
/// Base instructions for the session.
|
||||||
base_instructions: Option<String>,
|
base_instructions: String,
|
||||||
|
|
||||||
/// Compact prompt override.
|
/// Compact prompt override.
|
||||||
compact_prompt: Option<String>,
|
compact_prompt: Option<String>,
|
||||||
@@ -1555,13 +1538,9 @@ impl Session {
|
|||||||
conversation_id,
|
conversation_id,
|
||||||
forked_from_id,
|
forked_from_id,
|
||||||
session_source,
|
session_source,
|
||||||
session_configuration
|
BaseInstructions {
|
||||||
.base_instructions
|
text: session_configuration.base_instructions.clone(),
|
||||||
.clone()
|
},
|
||||||
.map(|text| BaseInstructions { text }),
|
|
||||||
session_configuration
|
|
||||||
.developer_instructions_override
|
|
||||||
.clone(),
|
|
||||||
session_configuration.dynamic_tools.clone(),
|
session_configuration.dynamic_tools.clone(),
|
||||||
if session_configuration.persist_extended_history {
|
if session_configuration.persist_extended_history {
|
||||||
EventPersistenceMode::Extended
|
EventPersistenceMode::Extended
|
||||||
@@ -2125,9 +2104,8 @@ impl Session {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if let Some(base_instructions) = session_configuration.base_instructions.clone() {
|
sess.schedule_startup_prewarm(session_configuration.base_instructions.clone())
|
||||||
sess.schedule_startup_prewarm(base_instructions).await;
|
.await;
|
||||||
}
|
|
||||||
let session_start_source = match &initial_history {
|
let session_start_source = match &initial_history {
|
||||||
InitialHistory::Resumed(_) => codex_hooks::SessionStartSource::Resume,
|
InitialHistory::Resumed(_) => codex_hooks::SessionStartSource::Resume,
|
||||||
InitialHistory::New | InitialHistory::Forked(_) => {
|
InitialHistory::New | InitialHistory::Forked(_) => {
|
||||||
@@ -2229,13 +2207,11 @@ impl Session {
|
|||||||
state.history.estimate_token_count(turn_context)
|
state.history.estimate_token_count(turn_context)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) async fn get_base_instructions(&self) -> Option<BaseInstructions> {
|
pub(crate) async fn get_base_instructions(&self) -> BaseInstructions {
|
||||||
let state = self.state.lock().await;
|
let state = self.state.lock().await;
|
||||||
state
|
BaseInstructions {
|
||||||
.session_configuration
|
text: state.session_configuration.base_instructions.clone(),
|
||||||
.base_instructions
|
}
|
||||||
.clone()
|
|
||||||
.map(|text| BaseInstructions { text })
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merges connector IDs into the session-level explicit connector selection.
|
// Merges connector IDs into the session-level explicit connector selection.
|
||||||
@@ -3639,11 +3615,7 @@ impl Session {
|
|||||||
state.reference_context_item(),
|
state.reference_context_item(),
|
||||||
state.previous_turn_settings(),
|
state.previous_turn_settings(),
|
||||||
state.session_configuration.collaboration_mode.clone(),
|
state.session_configuration.collaboration_mode.clone(),
|
||||||
state
|
state.session_configuration.base_instructions.clone(),
|
||||||
.session_configuration
|
|
||||||
.base_instructions
|
|
||||||
.clone()
|
|
||||||
.unwrap_or_default(),
|
|
||||||
state.session_configuration.session_source.clone(),
|
state.session_configuration.session_source.clone(),
|
||||||
)
|
)
|
||||||
};
|
};
|
||||||
@@ -3884,13 +3856,7 @@ impl Session {
|
|||||||
|
|
||||||
pub(crate) async fn recompute_token_usage(&self, turn_context: &TurnContext) {
|
pub(crate) async fn recompute_token_usage(&self, turn_context: &TurnContext) {
|
||||||
let history = self.clone_history().await;
|
let history = self.clone_history().await;
|
||||||
let empty_base_instructions = BaseInstructions {
|
let base_instructions = self.get_base_instructions().await;
|
||||||
text: String::new(),
|
|
||||||
};
|
|
||||||
let base_instructions = self
|
|
||||||
.get_base_instructions()
|
|
||||||
.await
|
|
||||||
.unwrap_or(empty_base_instructions);
|
|
||||||
let Some(estimated_total_tokens) =
|
let Some(estimated_total_tokens) =
|
||||||
history.estimate_token_count_with_base_instructions(&base_instructions)
|
history.estimate_token_count_with_base_instructions(&base_instructions)
|
||||||
else {
|
else {
|
||||||
@@ -6584,7 +6550,7 @@ pub(crate) fn build_prompt(
|
|||||||
input: Vec<ResponseItem>,
|
input: Vec<ResponseItem>,
|
||||||
router: &ToolRouter,
|
router: &ToolRouter,
|
||||||
turn_context: &TurnContext,
|
turn_context: &TurnContext,
|
||||||
base_instructions: Option<BaseInstructions>,
|
base_instructions: BaseInstructions,
|
||||||
) -> Prompt {
|
) -> Prompt {
|
||||||
let deferred_dynamic_tools = turn_context
|
let deferred_dynamic_tools = turn_context
|
||||||
.dynamic_tools
|
.dynamic_tools
|
||||||
|
|||||||
@@ -592,15 +592,11 @@ async fn get_base_instructions_no_user_content() {
|
|||||||
|
|
||||||
{
|
{
|
||||||
let mut state = session.state.lock().await;
|
let mut state = session.state.lock().await;
|
||||||
state.session_configuration.base_instructions =
|
state.session_configuration.base_instructions = model_info.base_instructions.clone();
|
||||||
Some(model_info.base_instructions.clone());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let base_instructions = session.get_base_instructions().await;
|
let base_instructions = session.get_base_instructions().await;
|
||||||
assert_eq!(
|
assert_eq!(base_instructions.text, model_info.base_instructions);
|
||||||
base_instructions.expect("base instructions").text,
|
|
||||||
model_info.base_instructions
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1096,7 +1092,7 @@ async fn recompute_token_usage_uses_session_base_instructions() {
|
|||||||
let override_instructions = "SESSION_OVERRIDE_INSTRUCTIONS_ONLY".repeat(120);
|
let override_instructions = "SESSION_OVERRIDE_INSTRUCTIONS_ONLY".repeat(120);
|
||||||
{
|
{
|
||||||
let mut state = session.state.lock().await;
|
let mut state = session.state.lock().await;
|
||||||
state.session_configuration.base_instructions = Some(override_instructions.clone());
|
state.session_configuration.base_instructions = override_instructions.clone();
|
||||||
}
|
}
|
||||||
|
|
||||||
let item = user_message("hello");
|
let item = user_message("hello");
|
||||||
@@ -1854,14 +1850,13 @@ async fn set_rate_limits_retains_previous_credits() {
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -1957,14 +1952,13 @@ async fn set_rate_limits_updates_plan_type_when_present() {
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -2229,8 +2223,7 @@ async fn attach_rollout_recorder(session: &Arc<Session>) -> PathBuf {
|
|||||||
ThreadId::default(),
|
ThreadId::default(),
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Exec,
|
SessionSource::Exec,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
@@ -2308,14 +2301,13 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -2572,14 +2564,13 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -2676,14 +2667,13 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -3518,14 +3508,13 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
|||||||
collaboration_mode,
|
collaboration_mode,
|
||||||
model_reasoning_summary: config.model_reasoning_summary,
|
model_reasoning_summary: config.model_reasoning_summary,
|
||||||
developer_instructions: config.developer_instructions.clone(),
|
developer_instructions: config.developer_instructions.clone(),
|
||||||
developer_instructions_override: config.developer_instructions_override.clone(),
|
|
||||||
user_instructions: config.user_instructions.clone(),
|
user_instructions: config.user_instructions.clone(),
|
||||||
service_tier: None,
|
service_tier: None,
|
||||||
personality: config.personality,
|
personality: config.personality,
|
||||||
base_instructions: config
|
base_instructions: config
|
||||||
.base_instructions
|
.base_instructions
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| Some(model_info.get_model_instructions(config.personality))),
|
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
|
||||||
compact_prompt: config.compact_prompt.clone(),
|
compact_prompt: config.compact_prompt.clone(),
|
||||||
approval_policy: config.permissions.approval_policy.clone(),
|
approval_policy: config.permissions.approval_policy.clone(),
|
||||||
approvals_reviewer: config.approvals_reviewer,
|
approvals_reviewer: config.approvals_reviewer,
|
||||||
@@ -4270,8 +4259,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline
|
|||||||
ThreadId::default(),
|
ThreadId::default(),
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Exec,
|
SessionSource::Exec,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
@@ -4368,8 +4356,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei
|
|||||||
ThreadId::default(),
|
ThreadId::default(),
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Exec,
|
SessionSource::Exec,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -76,16 +76,10 @@ async fn run_remote_compact_task_inner_impl(
|
|||||||
.await;
|
.await;
|
||||||
let mut history = sess.clone_history().await;
|
let mut history = sess.clone_history().await;
|
||||||
let base_instructions = sess.get_base_instructions().await;
|
let base_instructions = sess.get_base_instructions().await;
|
||||||
let token_count_base_instructions =
|
|
||||||
base_instructions
|
|
||||||
.clone()
|
|
||||||
.unwrap_or_else(|| BaseInstructions {
|
|
||||||
text: String::new(),
|
|
||||||
});
|
|
||||||
let deleted_items = trim_function_call_history_to_fit_context_window(
|
let deleted_items = trim_function_call_history_to_fit_context_window(
|
||||||
&mut history,
|
&mut history,
|
||||||
turn_context.as_ref(),
|
turn_context.as_ref(),
|
||||||
&token_count_base_instructions,
|
&base_instructions,
|
||||||
);
|
);
|
||||||
if deleted_items > 0 {
|
if deleted_items > 0 {
|
||||||
info!(
|
info!(
|
||||||
@@ -133,13 +127,8 @@ async fn run_remote_compact_task_inner_impl(
|
|||||||
)
|
)
|
||||||
.or_else(|err| async {
|
.or_else(|err| async {
|
||||||
let total_usage_breakdown = sess.get_total_token_usage_breakdown().await;
|
let total_usage_breakdown = sess.get_total_token_usage_breakdown().await;
|
||||||
let base_instruction_text = prompt
|
|
||||||
.base_instructions
|
|
||||||
.as_ref()
|
|
||||||
.map(|base_instructions| base_instructions.text.as_str())
|
|
||||||
.unwrap_or("");
|
|
||||||
let compact_request_log_data =
|
let compact_request_log_data =
|
||||||
build_compact_request_log_data(&prompt.input, base_instruction_text);
|
build_compact_request_log_data(&prompt.input, &prompt.base_instructions.text);
|
||||||
log_remote_compact_failure(
|
log_remote_compact_failure(
|
||||||
turn_context,
|
turn_context,
|
||||||
&compact_request_log_data,
|
&compact_request_log_data,
|
||||||
|
|||||||
@@ -4506,7 +4506,6 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
|||||||
experimental_realtime_ws_startup_context: None,
|
experimental_realtime_ws_startup_context: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
developer_instructions: None,
|
||||||
developer_instructions_override: None,
|
|
||||||
guardian_developer_instructions: None,
|
guardian_developer_instructions: None,
|
||||||
include_permissions_instructions: true,
|
include_permissions_instructions: true,
|
||||||
include_apps_instructions: true,
|
include_apps_instructions: true,
|
||||||
@@ -4652,7 +4651,6 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
|||||||
experimental_realtime_ws_startup_context: None,
|
experimental_realtime_ws_startup_context: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
developer_instructions: None,
|
||||||
developer_instructions_override: None,
|
|
||||||
guardian_developer_instructions: None,
|
guardian_developer_instructions: None,
|
||||||
include_permissions_instructions: true,
|
include_permissions_instructions: true,
|
||||||
include_apps_instructions: true,
|
include_apps_instructions: true,
|
||||||
@@ -4796,7 +4794,6 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
|||||||
experimental_realtime_ws_startup_context: None,
|
experimental_realtime_ws_startup_context: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
developer_instructions: None,
|
||||||
developer_instructions_override: None,
|
|
||||||
guardian_developer_instructions: None,
|
guardian_developer_instructions: None,
|
||||||
include_permissions_instructions: true,
|
include_permissions_instructions: true,
|
||||||
include_apps_instructions: true,
|
include_apps_instructions: true,
|
||||||
@@ -4926,7 +4923,6 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
|||||||
experimental_realtime_ws_startup_context: None,
|
experimental_realtime_ws_startup_context: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
developer_instructions: None,
|
||||||
developer_instructions_override: None,
|
|
||||||
guardian_developer_instructions: None,
|
guardian_developer_instructions: None,
|
||||||
include_permissions_instructions: true,
|
include_permissions_instructions: true,
|
||||||
include_apps_instructions: true,
|
include_apps_instructions: true,
|
||||||
|
|||||||
@@ -243,15 +243,11 @@ pub struct Config {
|
|||||||
pub user_instructions: Option<String>,
|
pub user_instructions: Option<String>,
|
||||||
|
|
||||||
/// Base instructions override.
|
/// Base instructions override.
|
||||||
pub base_instructions: Option<Option<String>>,
|
pub base_instructions: Option<String>,
|
||||||
|
|
||||||
/// Developer instructions override injected as a separate message.
|
/// Developer instructions override injected as a separate message.
|
||||||
pub developer_instructions: Option<String>,
|
pub developer_instructions: Option<String>,
|
||||||
|
|
||||||
/// Explicit developer instructions override, preserving `null` as distinct
|
|
||||||
/// from a missing override.
|
|
||||||
pub developer_instructions_override: Option<Option<String>>,
|
|
||||||
|
|
||||||
/// Guardian-specific developer instructions override from requirements.toml.
|
/// Guardian-specific developer instructions override from requirements.toml.
|
||||||
pub guardian_developer_instructions: Option<String>,
|
pub guardian_developer_instructions: Option<String>,
|
||||||
|
|
||||||
@@ -691,7 +687,7 @@ impl Config {
|
|||||||
model_context_window: self.model_context_window,
|
model_context_window: self.model_context_window,
|
||||||
model_auto_compact_token_limit: self.model_auto_compact_token_limit,
|
model_auto_compact_token_limit: self.model_auto_compact_token_limit,
|
||||||
tool_output_token_limit: self.tool_output_token_limit,
|
tool_output_token_limit: self.tool_output_token_limit,
|
||||||
base_instructions: self.base_instructions.clone().flatten(),
|
base_instructions: self.base_instructions.clone(),
|
||||||
personality_enabled: self.features.enabled(Feature::Personality),
|
personality_enabled: self.features.enabled(Feature::Personality),
|
||||||
model_supports_reasoning_summaries: self.model_supports_reasoning_summaries,
|
model_supports_reasoning_summaries: self.model_supports_reasoning_summaries,
|
||||||
model_catalog: self.model_catalog.clone(),
|
model_catalog: self.model_catalog.clone(),
|
||||||
@@ -1204,8 +1200,8 @@ pub struct ConfigOverrides {
|
|||||||
pub js_repl_node_path: Option<PathBuf>,
|
pub js_repl_node_path: Option<PathBuf>,
|
||||||
pub js_repl_node_module_dirs: Option<Vec<PathBuf>>,
|
pub js_repl_node_module_dirs: Option<Vec<PathBuf>>,
|
||||||
pub zsh_path: Option<PathBuf>,
|
pub zsh_path: Option<PathBuf>,
|
||||||
pub base_instructions: Option<Option<String>>,
|
pub base_instructions: Option<String>,
|
||||||
pub developer_instructions: Option<Option<String>>,
|
pub developer_instructions: Option<String>,
|
||||||
pub personality: Option<Personality>,
|
pub personality: Option<Personality>,
|
||||||
pub compact_prompt: Option<String>,
|
pub compact_prompt: Option<String>,
|
||||||
pub include_apply_patch_tool: Option<bool>,
|
pub include_apply_patch_tool: Option<bool>,
|
||||||
@@ -1764,10 +1760,8 @@ impl Config {
|
|||||||
.or(cfg.model_instructions_file.as_ref());
|
.or(cfg.model_instructions_file.as_ref());
|
||||||
let file_base_instructions =
|
let file_base_instructions =
|
||||||
Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?;
|
Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?;
|
||||||
let base_instructions = base_instructions.or_else(|| file_base_instructions.map(Some));
|
let base_instructions = base_instructions.or(file_base_instructions);
|
||||||
let developer_instructions_override = developer_instructions.clone();
|
let developer_instructions = developer_instructions.or(cfg.developer_instructions);
|
||||||
let developer_instructions =
|
|
||||||
developer_instructions.unwrap_or_else(|| cfg.developer_instructions.clone());
|
|
||||||
let include_permissions_instructions = config_profile
|
let include_permissions_instructions = config_profile
|
||||||
.include_permissions_instructions
|
.include_permissions_instructions
|
||||||
.or(cfg.include_permissions_instructions)
|
.or(cfg.include_permissions_instructions)
|
||||||
@@ -1950,7 +1944,6 @@ impl Config {
|
|||||||
base_instructions,
|
base_instructions,
|
||||||
personality,
|
personality,
|
||||||
developer_instructions,
|
developer_instructions,
|
||||||
developer_instructions_override,
|
|
||||||
compact_prompt,
|
compact_prompt,
|
||||||
commit_attribution,
|
commit_attribution,
|
||||||
include_permissions_instructions,
|
include_permissions_instructions,
|
||||||
|
|||||||
@@ -904,7 +904,7 @@ model_instructions_file = "child.txt"
|
|||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
config.base_instructions.as_ref().and_then(Option::as_deref),
|
config.base_instructions.as_deref(),
|
||||||
Some("child instructions")
|
Some("child instructions")
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -940,7 +940,7 @@ async fn cli_override_model_instructions_file_sets_base_instructions() -> std::i
|
|||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
config.base_instructions.as_ref().and_then(Option::as_deref),
|
config.base_instructions.as_deref(),
|
||||||
Some("cli override instructions")
|
Some("cli override instructions")
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -137,7 +137,7 @@ impl GuardianReviewSessionReuseKey {
|
|||||||
model_reasoning_summary: spawn_config.model_reasoning_summary,
|
model_reasoning_summary: spawn_config.model_reasoning_summary,
|
||||||
permissions: spawn_config.permissions.clone(),
|
permissions: spawn_config.permissions.clone(),
|
||||||
developer_instructions: spawn_config.developer_instructions.clone(),
|
developer_instructions: spawn_config.developer_instructions.clone(),
|
||||||
base_instructions: spawn_config.base_instructions.clone().flatten(),
|
base_instructions: spawn_config.base_instructions.clone(),
|
||||||
user_instructions: spawn_config.user_instructions.clone(),
|
user_instructions: spawn_config.user_instructions.clone(),
|
||||||
compact_prompt: spawn_config.compact_prompt.clone(),
|
compact_prompt: spawn_config.compact_prompt.clone(),
|
||||||
cwd: spawn_config.cwd.to_path_buf(),
|
cwd: spawn_config.cwd.to_path_buf(),
|
||||||
|
|||||||
@@ -336,9 +336,9 @@ mod job {
|
|||||||
}],
|
}],
|
||||||
tools: Vec::new(),
|
tools: Vec::new(),
|
||||||
parallel_tool_calls: false,
|
parallel_tool_calls: false,
|
||||||
base_instructions: Some(BaseInstructions {
|
base_instructions: BaseInstructions {
|
||||||
text: phase_one::PROMPT.to_string(),
|
text: phase_one::PROMPT.to_string(),
|
||||||
}),
|
},
|
||||||
personality: None,
|
personality: None,
|
||||||
output_schema: Some(output_schema()),
|
output_schema: Some(output_schema()),
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -43,7 +43,6 @@ async fn write_session_with_user_event(codex_home: &Path) -> io::Result<()> {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -217,9 +217,9 @@ async fn schedule_startup_prewarm_inner(
|
|||||||
Vec::new(),
|
Vec::new(),
|
||||||
startup_router.as_ref(),
|
startup_router.as_ref(),
|
||||||
startup_turn_context.as_ref(),
|
startup_turn_context.as_ref(),
|
||||||
Some(BaseInstructions {
|
BaseInstructions {
|
||||||
text: base_instructions,
|
text: base_instructions,
|
||||||
}),
|
},
|
||||||
);
|
);
|
||||||
let startup_turn_metadata_header = startup_turn_context
|
let startup_turn_metadata_header = startup_turn_context
|
||||||
.turn_metadata_state
|
.turn_metadata_state
|
||||||
|
|||||||
@@ -112,7 +112,7 @@ async fn start_review_conversation(
|
|||||||
let _ = sub_agent_config.features.disable(Feature::Collab);
|
let _ = sub_agent_config.features.disable(Feature::Collab);
|
||||||
|
|
||||||
// Set explicit review rubric for the sub-agent
|
// Set explicit review rubric for the sub-agent
|
||||||
sub_agent_config.base_instructions = Some(Some(crate::REVIEW_PROMPT.to_string()));
|
sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string());
|
||||||
sub_agent_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
|
sub_agent_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
|
||||||
|
|
||||||
let model = config
|
let model = config
|
||||||
|
|||||||
@@ -535,7 +535,7 @@ async fn build_runner_options(
|
|||||||
let max_concurrency =
|
let max_concurrency =
|
||||||
normalize_concurrency(requested_concurrency, turn.config.agent_max_threads);
|
normalize_concurrency(requested_concurrency, turn.config.agent_max_threads);
|
||||||
let base_instructions = session.get_base_instructions().await;
|
let base_instructions = session.get_base_instructions().await;
|
||||||
let spawn_config = build_agent_spawn_config(base_instructions.as_ref(), turn.as_ref())?;
|
let spawn_config = build_agent_spawn_config(&base_instructions, turn.as_ref())?;
|
||||||
Ok(JobRunnerOptions {
|
Ok(JobRunnerOptions {
|
||||||
max_concurrency,
|
max_concurrency,
|
||||||
spawn_config,
|
spawn_config,
|
||||||
|
|||||||
@@ -59,10 +59,8 @@ impl ToolHandler for Handler {
|
|||||||
.into(),
|
.into(),
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
let mut config = build_agent_spawn_config(
|
let mut config =
|
||||||
session.get_base_instructions().await.as_ref(),
|
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
|
||||||
turn.as_ref(),
|
|
||||||
)?;
|
|
||||||
apply_requested_spawn_agent_model_overrides(
|
apply_requested_spawn_agent_model_overrides(
|
||||||
&session,
|
&session,
|
||||||
turn.as_ref(),
|
turn.as_ref(),
|
||||||
|
|||||||
@@ -201,12 +201,11 @@ pub(crate) fn parse_collab_input(
|
|||||||
/// skipping this helper and cloning stale config state directly can send the child agent out with
|
/// skipping this helper and cloning stale config state directly can send the child agent out with
|
||||||
/// the wrong provider or runtime policy.
|
/// the wrong provider or runtime policy.
|
||||||
pub(crate) fn build_agent_spawn_config(
|
pub(crate) fn build_agent_spawn_config(
|
||||||
base_instructions: Option<&BaseInstructions>,
|
base_instructions: &BaseInstructions,
|
||||||
turn: &TurnContext,
|
turn: &TurnContext,
|
||||||
) -> Result<Config, FunctionCallError> {
|
) -> Result<Config, FunctionCallError> {
|
||||||
let mut config = build_agent_shared_config(turn)?;
|
let mut config = build_agent_shared_config(turn)?;
|
||||||
config.base_instructions =
|
config.base_instructions = Some(base_instructions.text.clone());
|
||||||
Some(base_instructions.map(|base_instructions| base_instructions.text.clone()));
|
|
||||||
Ok(config)
|
Ok(config)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3255,9 +3255,9 @@ async fn build_agent_spawn_config_uses_turn_context_values() {
|
|||||||
.set(AskForApproval::OnRequest)
|
.set(AskForApproval::OnRequest)
|
||||||
.expect("approval policy set");
|
.expect("approval policy set");
|
||||||
|
|
||||||
let config = build_agent_spawn_config(Some(&base_instructions), &turn).expect("spawn config");
|
let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config");
|
||||||
let mut expected = (*turn.config).clone();
|
let mut expected = (*turn.config).clone();
|
||||||
expected.base_instructions = Some(Some(base_instructions.text));
|
expected.base_instructions = Some(base_instructions.text);
|
||||||
expected.model = Some(turn.model_info.slug.clone());
|
expected.model = Some(turn.model_info.slug.clone());
|
||||||
expected.model_provider = turn.provider.clone();
|
expected.model_provider = turn.provider.clone();
|
||||||
expected.model_reasoning_effort = turn.reasoning_effort;
|
expected.model_reasoning_effort = turn.reasoning_effort;
|
||||||
@@ -3293,7 +3293,7 @@ async fn build_agent_spawn_config_preserves_base_user_instructions() {
|
|||||||
text: "base".to_string(),
|
text: "base".to_string(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let config = build_agent_spawn_config(Some(&base_instructions), &turn).expect("spawn config");
|
let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config");
|
||||||
|
|
||||||
assert_eq!(config.user_instructions, base_config.user_instructions);
|
assert_eq!(config.user_instructions, base_config.user_instructions);
|
||||||
}
|
}
|
||||||
@@ -3302,7 +3302,7 @@ async fn build_agent_spawn_config_preserves_base_user_instructions() {
|
|||||||
async fn build_agent_resume_config_clears_base_instructions() {
|
async fn build_agent_resume_config_clears_base_instructions() {
|
||||||
let (_session, mut turn) = make_session_and_context().await;
|
let (_session, mut turn) = make_session_and_context().await;
|
||||||
let mut base_config = (*turn.config).clone();
|
let mut base_config = (*turn.config).clone();
|
||||||
base_config.base_instructions = Some(Some("caller-base".to_string()));
|
base_config.base_instructions = Some("caller-base".to_string());
|
||||||
turn.config = Arc::new(base_config);
|
turn.config = Arc::new(base_config);
|
||||||
turn.approval_policy
|
turn.approval_policy
|
||||||
.set(AskForApproval::OnRequest)
|
.set(AskForApproval::OnRequest)
|
||||||
|
|||||||
@@ -69,10 +69,8 @@ impl ToolHandler for Handler {
|
|||||||
.into(),
|
.into(),
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
let mut config = build_agent_spawn_config(
|
let mut config =
|
||||||
session.get_base_instructions().await.as_ref(),
|
build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?;
|
||||||
turn.as_ref(),
|
|
||||||
)?;
|
|
||||||
apply_requested_spawn_agent_model_overrides(
|
apply_requested_spawn_agent_model_overrides(
|
||||||
&session,
|
&session,
|
||||||
turn.as_ref(),
|
turn.as_ref(),
|
||||||
|
|||||||
@@ -932,7 +932,7 @@ async fn includes_base_instructions_override_in_request() {
|
|||||||
let mut builder = test_codex()
|
let mut builder = test_codex()
|
||||||
.with_auth(CodexAuth::from_api_key("Test API Key"))
|
.with_auth(CodexAuth::from_api_key("Test API Key"))
|
||||||
.with_config(|config| {
|
.with_config(|config| {
|
||||||
config.base_instructions = Some(Some("test instructions".to_string()));
|
config.base_instructions = Some("test instructions".to_string());
|
||||||
});
|
});
|
||||||
let codex = builder
|
let codex = builder
|
||||||
.build(&server)
|
.build(&server)
|
||||||
@@ -964,47 +964,6 @@ async fn includes_base_instructions_override_in_request() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
||||||
async fn omits_explicit_null_base_instructions_from_request() {
|
|
||||||
skip_if_no_network!();
|
|
||||||
// Mock server
|
|
||||||
let server = MockServer::start().await;
|
|
||||||
let resp_mock = mount_sse_once(
|
|
||||||
&server,
|
|
||||||
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
|
|
||||||
let mut builder = test_codex()
|
|
||||||
.with_auth(CodexAuth::from_api_key("Test API Key"))
|
|
||||||
.with_config(|config| {
|
|
||||||
config.base_instructions = Some(None);
|
|
||||||
});
|
|
||||||
let codex = builder
|
|
||||||
.build(&server)
|
|
||||||
.await
|
|
||||||
.expect("create new conversation")
|
|
||||||
.codex;
|
|
||||||
|
|
||||||
codex
|
|
||||||
.submit(Op::UserInput {
|
|
||||||
items: vec![UserInput::Text {
|
|
||||||
text: "hello".into(),
|
|
||||||
text_elements: Vec::new(),
|
|
||||||
}],
|
|
||||||
final_output_json_schema: None,
|
|
||||||
})
|
|
||||||
.await
|
|
||||||
.unwrap();
|
|
||||||
|
|
||||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
|
||||||
|
|
||||||
let request = resp_mock.single_request();
|
|
||||||
let request_body = request.body_json();
|
|
||||||
|
|
||||||
assert_eq!(request_body.get("instructions"), None);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn chatgpt_auth_sends_correct_request() {
|
async fn chatgpt_auth_sends_correct_request() {
|
||||||
skip_if_no_network!();
|
skip_if_no_network!();
|
||||||
|
|||||||
@@ -1659,9 +1659,9 @@ fn prompt_with_input(input: Vec<ResponseItem>) -> Prompt {
|
|||||||
|
|
||||||
fn prompt_with_input_and_instructions(input: Vec<ResponseItem>, instructions: &str) -> Prompt {
|
fn prompt_with_input_and_instructions(input: Vec<ResponseItem>, instructions: &str) -> Prompt {
|
||||||
let mut prompt = prompt_with_input(input);
|
let mut prompt = prompt_with_input(input);
|
||||||
prompt.base_instructions = Some(BaseInstructions {
|
prompt.base_instructions = BaseInstructions {
|
||||||
text: instructions.to_string(),
|
text: instructions.to_string(),
|
||||||
});
|
};
|
||||||
prompt
|
prompt
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -869,7 +869,7 @@ async fn remote_compact_trim_estimate_uses_session_base_instructions() -> Result
|
|||||||
let override_base_instructions = override_base_instructions.clone();
|
let override_base_instructions = override_base_instructions.clone();
|
||||||
move |config| {
|
move |config| {
|
||||||
config.model_context_window = Some(override_context_window);
|
config.model_context_window = Some(override_context_window);
|
||||||
config.base_instructions = Some(Some(override_base_instructions));
|
config.base_instructions = Some(override_base_instructions);
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -66,7 +66,7 @@ async fn base_instructions_override_disables_personality_template() {
|
|||||||
.enable(Feature::Personality)
|
.enable(Feature::Personality)
|
||||||
.expect("test config should allow feature update");
|
.expect("test config should allow feature update");
|
||||||
config.personality = Some(Personality::Friendly);
|
config.personality = Some(Personality::Friendly);
|
||||||
config.base_instructions = Some(Some("override instructions".to_string()));
|
config.base_instructions = Some("override instructions".to_string());
|
||||||
|
|
||||||
let model_info =
|
let model_info =
|
||||||
codex_core::test_support::construct_model_info_offline("gpt-5.2-codex", &config);
|
codex_core::test_support::construct_model_info_offline("gpt-5.2-codex", &config);
|
||||||
|
|||||||
@@ -71,7 +71,6 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
@@ -117,7 +116,6 @@ async fn write_rollout_with_meta_only(dir: &Path, thread_id: ThreadId) -> io::Re
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -173,8 +173,7 @@ async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()>
|
|||||||
thread_id,
|
thread_id,
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Exec,
|
SessionSource::Exec,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -146,7 +146,6 @@ async fn backfill_scans_existing_rollouts() -> Result<()> {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: Some(dynamic_tools_for_hook),
|
dynamic_tools: Some(dynamic_tools_for_hook),
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -84,7 +84,7 @@ async fn continue_after_stream_error() {
|
|||||||
|
|
||||||
let TestCodex { codex, .. } = test_codex()
|
let TestCodex { codex, .. } = test_codex()
|
||||||
.with_config(move |config| {
|
.with_config(move |config| {
|
||||||
config.base_instructions = Some(Some("You are a helpful assistant".to_string()));
|
config.base_instructions = Some("You are a helpful assistant".to_string());
|
||||||
config.model_provider = provider;
|
config.model_provider = provider;
|
||||||
})
|
})
|
||||||
.build(&server)
|
.build(&server)
|
||||||
|
|||||||
@@ -179,8 +179,8 @@ impl CodexToolCallParam {
|
|||||||
codex_self_exe: arg0_paths.codex_self_exe.clone(),
|
codex_self_exe: arg0_paths.codex_self_exe.clone(),
|
||||||
codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(),
|
codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(),
|
||||||
main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(),
|
main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(),
|
||||||
base_instructions: base_instructions.map(Some),
|
base_instructions,
|
||||||
developer_instructions: developer_instructions.map(Some),
|
developer_instructions,
|
||||||
compact_prompt,
|
compact_prompt,
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -23,5 +23,4 @@ pub mod plan_tool;
|
|||||||
pub mod protocol;
|
pub mod protocol;
|
||||||
pub mod request_permissions;
|
pub mod request_permissions;
|
||||||
pub mod request_user_input;
|
pub mod request_user_input;
|
||||||
mod serde_helpers;
|
|
||||||
pub mod user_input;
|
pub mod user_input;
|
||||||
@@ -48,8 +48,6 @@ use crate::plan_tool::UpdatePlanArgs;
|
|||||||
use crate::request_permissions::RequestPermissionsEvent;
|
use crate::request_permissions::RequestPermissionsEvent;
|
||||||
use crate::request_permissions::RequestPermissionsResponse;
|
use crate::request_permissions::RequestPermissionsResponse;
|
||||||
use crate::request_user_input::RequestUserInputResponse;
|
use crate::request_user_input::RequestUserInputResponse;
|
||||||
use crate::serde_helpers::deserialize_double_option;
|
|
||||||
use crate::serde_helpers::serialize_double_option;
|
|
||||||
use crate::user_input::UserInput;
|
use crate::user_input::UserInput;
|
||||||
use codex_git_utils::GitSha;
|
use codex_git_utils::GitSha;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
@@ -2320,7 +2318,7 @@ impl InitialHistory {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn get_base_instructions(&self) -> Option<Option<BaseInstructions>> {
|
pub fn get_base_instructions(&self) -> Option<BaseInstructions> {
|
||||||
// TODO: SessionMeta should (in theory) always be first in the history, so we can probably only check the first item?
|
// TODO: SessionMeta should (in theory) always be first in the history, so we can probably only check the first item?
|
||||||
match self {
|
match self {
|
||||||
InitialHistory::New => None,
|
InitialHistory::New => None,
|
||||||
@@ -2337,26 +2335,6 @@ impl InitialHistory {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn get_developer_instructions(&self) -> Option<Option<String>> {
|
|
||||||
match self {
|
|
||||||
InitialHistory::New => None,
|
|
||||||
InitialHistory::Resumed(resumed) => {
|
|
||||||
resumed.history.iter().find_map(|item| match item {
|
|
||||||
RolloutItem::SessionMeta(meta_line) => {
|
|
||||||
meta_line.meta.developer_instructions.clone()
|
|
||||||
}
|
|
||||||
_ => None,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
InitialHistory::Forked(items) => items.iter().find_map(|item| match item {
|
|
||||||
RolloutItem::SessionMeta(meta_line) => {
|
|
||||||
meta_line.meta.developer_instructions.clone()
|
|
||||||
}
|
|
||||||
_ => None,
|
|
||||||
}),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn get_dynamic_tools(&self) -> Option<Vec<DynamicToolSpec>> {
|
pub fn get_dynamic_tools(&self) -> Option<Vec<DynamicToolSpec>> {
|
||||||
match self {
|
match self {
|
||||||
InitialHistory::New => None,
|
InitialHistory::New => None,
|
||||||
@@ -2548,20 +2526,7 @@ pub struct SessionMeta {
|
|||||||
/// base_instructions for the session. This *should* always be present when creating a new session,
|
/// base_instructions for the session. This *should* always be present when creating a new session,
|
||||||
/// but may be missing for older sessions. If not present, fall back to rendering the base_instructions
|
/// but may be missing for older sessions. If not present, fall back to rendering the base_instructions
|
||||||
/// from ModelsManager.
|
/// from ModelsManager.
|
||||||
#[serde(
|
pub base_instructions: Option<BaseInstructions>,
|
||||||
default,
|
|
||||||
deserialize_with = "deserialize_double_option",
|
|
||||||
serialize_with = "serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
pub base_instructions: Option<Option<BaseInstructions>>,
|
|
||||||
#[serde(
|
|
||||||
default,
|
|
||||||
deserialize_with = "deserialize_double_option",
|
|
||||||
serialize_with = "serialize_double_option",
|
|
||||||
skip_serializing_if = "Option::is_none"
|
|
||||||
)]
|
|
||||||
pub developer_instructions: Option<Option<String>>,
|
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub dynamic_tools: Option<Vec<DynamicToolSpec>>,
|
pub dynamic_tools: Option<Vec<DynamicToolSpec>>,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
@@ -2583,7 +2548,6 @@ impl Default for SessionMeta {
|
|||||||
agent_path: None,
|
agent_path: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,23 +0,0 @@
|
|||||||
use serde::Deserialize;
|
|
||||||
use serde::Deserializer;
|
|
||||||
use serde::Serialize;
|
|
||||||
use serde::Serializer;
|
|
||||||
|
|
||||||
pub fn deserialize_double_option<'de, T, D>(deserializer: D) -> Result<Option<Option<T>>, D::Error>
|
|
||||||
where
|
|
||||||
T: Deserialize<'de>,
|
|
||||||
D: Deserializer<'de>,
|
|
||||||
{
|
|
||||||
serde_with::rust::double_option::deserialize(deserializer)
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn serialize_double_option<T, S>(
|
|
||||||
value: &Option<Option<T>>,
|
|
||||||
serializer: S,
|
|
||||||
) -> Result<S::Ok, S::Error>
|
|
||||||
where
|
|
||||||
T: Serialize,
|
|
||||||
S: Serializer,
|
|
||||||
{
|
|
||||||
serde_with::rust::double_option::serialize(value, serializer)
|
|
||||||
}
|
|
||||||
@@ -56,7 +56,6 @@ async fn extract_metadata_from_rollout_uses_session_meta() {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("openai".to_string()),
|
model_provider: Some("openai".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
@@ -108,7 +107,6 @@ async fn extract_metadata_from_rollout_returns_latest_memory_mode() {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("openai".to_string()),
|
model_provider: Some("openai".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
@@ -371,7 +369,6 @@ fn write_rollout_in_sessions_with_cwd(
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("test-provider".to_string()),
|
model_provider: Some("test-provider".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -81,8 +81,7 @@ pub enum RolloutRecorderParams {
|
|||||||
conversation_id: ThreadId,
|
conversation_id: ThreadId,
|
||||||
forked_from_id: Option<ThreadId>,
|
forked_from_id: Option<ThreadId>,
|
||||||
source: SessionSource,
|
source: SessionSource,
|
||||||
base_instructions: Option<BaseInstructions>,
|
base_instructions: BaseInstructions,
|
||||||
developer_instructions: Option<Option<String>>,
|
|
||||||
dynamic_tools: Vec<DynamicToolSpec>,
|
dynamic_tools: Vec<DynamicToolSpec>,
|
||||||
event_persistence_mode: EventPersistenceMode,
|
event_persistence_mode: EventPersistenceMode,
|
||||||
},
|
},
|
||||||
@@ -111,8 +110,7 @@ impl RolloutRecorderParams {
|
|||||||
conversation_id: ThreadId,
|
conversation_id: ThreadId,
|
||||||
forked_from_id: Option<ThreadId>,
|
forked_from_id: Option<ThreadId>,
|
||||||
source: SessionSource,
|
source: SessionSource,
|
||||||
base_instructions: Option<BaseInstructions>,
|
base_instructions: BaseInstructions,
|
||||||
developer_instructions: Option<Option<String>>,
|
|
||||||
dynamic_tools: Vec<DynamicToolSpec>,
|
dynamic_tools: Vec<DynamicToolSpec>,
|
||||||
event_persistence_mode: EventPersistenceMode,
|
event_persistence_mode: EventPersistenceMode,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
@@ -121,7 +119,6 @@ impl RolloutRecorderParams {
|
|||||||
forked_from_id,
|
forked_from_id,
|
||||||
source,
|
source,
|
||||||
base_instructions,
|
base_instructions,
|
||||||
developer_instructions,
|
|
||||||
dynamic_tools,
|
dynamic_tools,
|
||||||
event_persistence_mode,
|
event_persistence_mode,
|
||||||
}
|
}
|
||||||
@@ -383,7 +380,6 @@ impl RolloutRecorder {
|
|||||||
forked_from_id,
|
forked_from_id,
|
||||||
source,
|
source,
|
||||||
base_instructions,
|
base_instructions,
|
||||||
developer_instructions,
|
|
||||||
dynamic_tools,
|
dynamic_tools,
|
||||||
event_persistence_mode,
|
event_persistence_mode,
|
||||||
} => {
|
} => {
|
||||||
@@ -413,7 +409,6 @@ impl RolloutRecorder {
|
|||||||
source,
|
source,
|
||||||
model_provider: Some(config.model_provider_id().to_string()),
|
model_provider: Some(config.model_provider_id().to_string()),
|
||||||
base_instructions: Some(base_instructions),
|
base_instructions: Some(base_instructions),
|
||||||
developer_instructions,
|
|
||||||
dynamic_tools: if dynamic_tools.is_empty() {
|
dynamic_tools: if dynamic_tools.is_empty() {
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -73,8 +73,7 @@ async fn recorder_materializes_only_after_explicit_persist() -> std::io::Result<
|
|||||||
thread_id,
|
thread_id,
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Exec,
|
SessionSource::Exec,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
@@ -167,8 +166,7 @@ async fn metadata_irrelevant_events_touch_state_db_updated_at() -> std::io::Resu
|
|||||||
thread_id,
|
thread_id,
|
||||||
/*forked_from_id*/ None,
|
/*forked_from_id*/ None,
|
||||||
SessionSource::Cli,
|
SessionSource::Cli,
|
||||||
Some(BaseInstructions::default()),
|
BaseInstructions::default(),
|
||||||
/*developer_instructions*/ None,
|
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
EventPersistenceMode::Limited,
|
EventPersistenceMode::Limited,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -977,7 +977,7 @@ async fn test_get_thread_contents() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn test_base_instructions_missing_in_meta_stays_missing() {
|
async fn test_base_instructions_missing_in_meta_defaults_to_null() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
let home = temp.path();
|
let home = temp.path();
|
||||||
|
|
||||||
@@ -1011,7 +1011,10 @@ async fn test_base_instructions_missing_in_meta_stays_missing() {
|
|||||||
.await
|
.await
|
||||||
.expect("session meta head");
|
.expect("session meta head");
|
||||||
let first = head.first().expect("first head entry");
|
let first = head.first().expect("first head entry");
|
||||||
assert_eq!(first.get("base_instructions"), None);
|
assert_eq!(
|
||||||
|
first.get("base_instructions"),
|
||||||
|
Some(&serde_json::Value::Null)
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -1140,7 +1143,6 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("test-provider".into()),
|
model_provider: Some("test-provider".into()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -257,7 +257,6 @@ mod tests {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("openai".to_string()),
|
model_provider: Some("openai".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
@@ -385,7 +384,6 @@ mod tests {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: Some("openai".to_string()),
|
model_provider: Some("openai".to_string()),
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1030,7 +1030,6 @@ mod tests {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: Some("polluted".to_string()),
|
memory_mode: Some("polluted".to_string()),
|
||||||
},
|
},
|
||||||
@@ -1089,7 +1088,6 @@ mod tests {
|
|||||||
agent_role: None,
|
agent_role: None,
|
||||||
model_provider: None,
|
model_provider: None,
|
||||||
base_instructions: None,
|
base_instructions: None,
|
||||||
developer_instructions: None,
|
|
||||||
dynamic_tools: None,
|
dynamic_tools: None,
|
||||||
memory_mode: None,
|
memory_mode: None,
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in new issue
Block a user