mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Add optional names to untrusted additional context (#57181)
## What changed
- Add an optional `name` to `AdditionalContextEntry` across the protocol, app-server schemas, and generated clients.
- Accept names matching `[a-z][a-z0-9_]{0,63}` only for `untrusted` context. Reject invalid names and names on `application` context before admitting turn input, returning invalid-request errors from `turn/start` and `turn/steer`.
- Preserve existing model-input rendering and deduplicate context when only its name changes.
## Testing
Add coverage for valid name boundaries, callers that omit `name`, invalid-name rejection without admitting input, and deduplication across name changes.
GitOrigin-RevId: 2caffa6fddd40ddb178733b335f520f317239edd
This commit is contained in:
@@ -17,6 +17,13 @@
|
||||
"kind": {
|
||||
"$ref": "#/definitions/AdditionalContextKind"
|
||||
},
|
||||
"name": {
|
||||
"description": "Optional name for untrusted context, used to label its tool output.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"value": {
|
||||
"type": "string"
|
||||
}
|
||||
|
||||
+7
@@ -7034,6 +7034,13 @@
|
||||
"kind": {
|
||||
"$ref": "#/definitions/v2/AdditionalContextKind"
|
||||
},
|
||||
"name": {
|
||||
"description": "Optional name for untrusted context, used to label its tool output.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"value": {
|
||||
"type": "string"
|
||||
}
|
||||
|
||||
+7
@@ -269,6 +269,13 @@
|
||||
"kind": {
|
||||
"$ref": "#/definitions/AdditionalContextKind"
|
||||
},
|
||||
"name": {
|
||||
"description": "Optional name for untrusted context, used to label its tool output.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"value": {
|
||||
"type": "string"
|
||||
}
|
||||
|
||||
@@ -10,6 +10,13 @@
|
||||
"kind": {
|
||||
"$ref": "#/definitions/AdditionalContextKind"
|
||||
},
|
||||
"name": {
|
||||
"description": "Optional name for untrusted context, used to label its tool output.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"value": {
|
||||
"type": "string"
|
||||
}
|
||||
|
||||
@@ -6,6 +6,13 @@
|
||||
"kind": {
|
||||
"$ref": "#/definitions/AdditionalContextKind"
|
||||
},
|
||||
"name": {
|
||||
"description": "Optional name for untrusted context, used to label its tool output.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"value": {
|
||||
"type": "string"
|
||||
}
|
||||
|
||||
BIN
Binary file not shown.
BIN
Binary file not shown.
+5
-1
@@ -3,4 +3,8 @@
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
import type { AdditionalContextKind } from "./AdditionalContextKind";
|
||||
|
||||
export type AdditionalContextEntry = { value: string, kind: AdditionalContextKind, };
|
||||
export type AdditionalContextEntry = { value: string, kind: AdditionalContextKind,
|
||||
/**
|
||||
* Optional name for untrusted context, used to label its tool output.
|
||||
*/
|
||||
name?: string | null, };
|
||||
|
||||
@@ -2262,14 +2262,14 @@ mod tests {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only allow "?: T | null" in objects representing JSON-RPC requests,
|
||||
// which we assume are called "*Params".
|
||||
// Allow "?: T | null" in JSON-RPC request objects and nested request types.
|
||||
let allow_optional_nullable = path
|
||||
.file_stem()
|
||||
.and_then(|stem| stem.to_str())
|
||||
.is_some_and(|stem| {
|
||||
stem.ends_with("Params")
|
||||
|| stem == "InitializeCapabilities"
|
||||
|| stem == "AdditionalContextEntry"
|
||||
|| matches!(
|
||||
stem,
|
||||
"CollabAgentRef"
|
||||
|
||||
@@ -116,6 +116,9 @@ pub enum AdditionalContextKind {
|
||||
pub struct AdditionalContextEntry {
|
||||
pub value: String,
|
||||
pub kind: AdditionalContextKind,
|
||||
/// Optional name for untrusted context, used to label its tool output.
|
||||
#[ts(optional = nullable)]
|
||||
pub name: Option<String>,
|
||||
}
|
||||
|
||||
/// Requested cyber treatment for an OpenAI model turn.
|
||||
|
||||
@@ -444,6 +444,8 @@ use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AdditionalContextEntry as CoreAdditionalContextEntry;
|
||||
use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind;
|
||||
use codex_protocol::protocol::AgentStatus;
|
||||
use codex_protocol::protocol::ConversationAudioParams;
|
||||
use codex_protocol::protocol::ConversationSpeechParams;
|
||||
|
||||
@@ -11,8 +11,6 @@ use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ImageReference as CoreImageReference;
|
||||
use codex_protocol::protocol::AdditionalContextEntry as CoreAdditionalContextEntry;
|
||||
use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind;
|
||||
use codex_protocol::protocol::TurnEnvironmentRequests;
|
||||
use codex_protocol::protocol::TurnSettingsUpdate;
|
||||
use codex_protocol::protocol::TurnSettingsUpdateOutcome;
|
||||
@@ -104,18 +102,15 @@ fn map_additional_context(
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(|(key, entry)| {
|
||||
(
|
||||
key,
|
||||
CoreAdditionalContextEntry {
|
||||
value: entry.value,
|
||||
kind: match entry.kind {
|
||||
AdditionalContextKind::Untrusted => CoreAdditionalContextKind::Untrusted,
|
||||
AdditionalContextKind::Application => {
|
||||
CoreAdditionalContextKind::Application
|
||||
}
|
||||
},
|
||||
let entry = CoreAdditionalContextEntry {
|
||||
value: entry.value,
|
||||
kind: match entry.kind {
|
||||
AdditionalContextKind::Untrusted => CoreAdditionalContextKind::Untrusted,
|
||||
AdditionalContextKind::Application => CoreAdditionalContextKind::Application,
|
||||
},
|
||||
)
|
||||
name: entry.name,
|
||||
};
|
||||
(key, entry)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -581,6 +576,15 @@ impl TurnRequestProcessor {
|
||||
);
|
||||
return Err(error);
|
||||
}
|
||||
// Validate before changing the thread's live MCP policy.
|
||||
let additional_context = map_additional_context(params.additional_context);
|
||||
additional_context
|
||||
.values()
|
||||
.try_for_each(CoreAdditionalContextEntry::validate)
|
||||
.map_err(invalid_request)
|
||||
.inspect_err(|error| {
|
||||
self.track_error_response(&request_id, error, /*error_type*/ None);
|
||||
})?;
|
||||
Self::set_app_server_client_info(
|
||||
thread.as_ref(),
|
||||
app_server_client_name,
|
||||
@@ -596,7 +600,6 @@ impl TurnRequestProcessor {
|
||||
let environment_requests =
|
||||
resolve_turn_environment_requests(self.thread_manager.as_ref(), params.environments)?;
|
||||
|
||||
let additional_context = map_additional_context(params.additional_context);
|
||||
let turn_has_input = !params.input.is_empty();
|
||||
let input = if let Some(tool_output) = params.tool_output {
|
||||
let item = ResponseItem::FunctionCallOutput {
|
||||
@@ -679,7 +682,10 @@ impl TurnRequestProcessor {
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
let error = internal_error(format!("failed to submit turn input: {err}"));
|
||||
let error = match err.details() {
|
||||
CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()),
|
||||
_ => internal_error(format!("failed to submit turn input: {err}")),
|
||||
};
|
||||
self.track_error_response(&request_id, &error, /*error_type*/ None);
|
||||
error
|
||||
})?;
|
||||
@@ -1096,7 +1102,10 @@ impl TurnRequestProcessor {
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
let error = internal_error(format!("failed to steer turn: {err}"));
|
||||
let error = match err.details() {
|
||||
CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()),
|
||||
_ => internal_error(format!("failed to steer turn: {err}")),
|
||||
};
|
||||
self.track_error_response(request_id, &error, /*error_type*/ None);
|
||||
error
|
||||
})?;
|
||||
|
||||
@@ -196,6 +196,7 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history(
|
||||
AdditionalContextEntry {
|
||||
value: application_context_text.to_string(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
name: None,
|
||||
},
|
||||
),
|
||||
(
|
||||
@@ -203,6 +204,7 @@ async fn thread_inject_items_adds_raw_response_items_to_thread_history(
|
||||
AdditionalContextEntry {
|
||||
value: untrusted_context_text.to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
name: None,
|
||||
},
|
||||
),
|
||||
])),
|
||||
|
||||
@@ -790,8 +790,13 @@ async fn turn_start_steers_active_turn_and_returns_active_turn_id() -> Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(Some("browser"); "named_context")]
|
||||
#[test_case::test_case(Some("a"); "minimum_length")]
|
||||
#[test_case::test_case(Some("browser_2"); "digits_and_underscore")]
|
||||
#[test_case::test_case(Some(&"a".repeat(64)); "maximum_length")]
|
||||
#[test_case::test_case(None; "legacy_caller")]
|
||||
#[tokio::test]
|
||||
async fn turn_start_additional_context_flows_to_model_input() -> Result<()> {
|
||||
async fn turn_start_additional_context_flows_to_model_input(name: Option<&str>) -> Result<()> {
|
||||
let responses = vec![create_final_assistant_message_sse_response("Done")?];
|
||||
let server = create_mock_responses_server_sequence_unchecked(responses).await;
|
||||
|
||||
@@ -810,27 +815,75 @@ async fn turn_start_additional_context_flows_to_model_input() -> Result<()> {
|
||||
})
|
||||
.await?;
|
||||
|
||||
let _: TurnStartResponse = mcp
|
||||
.request(|request_id| ClientRequest::TurnStart {
|
||||
request_id,
|
||||
params: TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
client_user_message_id: None,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "inspect tab".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
additional_context: Some(HashMap::from([(
|
||||
"custom_source".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "source value".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
},
|
||||
let rejected_request = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "rejected input".to_owned(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
additional_context: Some(HashMap::from([(
|
||||
"application".to_owned(),
|
||||
AdditionalContextEntry {
|
||||
value: "rejected context".to_owned(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
name: Some("browser".to_owned()),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(rejected_request)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE);
|
||||
|
||||
let mut params = serde_json::to_value(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
client_user_message_id: None,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "inspect tab".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
additional_context: Some(HashMap::from([(
|
||||
"custom_source".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "source value".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
name: name.map(str::to_owned),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
})?;
|
||||
let entry = params["additionalContext"]["custom_source"]
|
||||
.as_object_mut()
|
||||
.context("additional context entry")?;
|
||||
if name.is_none() {
|
||||
entry.remove("name");
|
||||
}
|
||||
let mut malformed_params = params.clone();
|
||||
malformed_params["additionalContext"]["custom_source"]["name"] = json!("agent.browser");
|
||||
let rejected_request = mcp
|
||||
.send_request("turn/start", Some(malformed_params))
|
||||
.await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(rejected_request)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(
|
||||
error.error,
|
||||
codex_app_server_protocol::JSONRPCErrorError {
|
||||
code: INVALID_REQUEST_ERROR_CODE,
|
||||
message: "additional context name must match [a-z][a-z0-9_]{0,63}".to_owned(),
|
||||
data: None,
|
||||
}
|
||||
);
|
||||
|
||||
let request_id = mcp.send_request("turn/start", Some(params)).await?;
|
||||
let _: TurnStartResponse = mcp.read_response(request_id).await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
@@ -848,10 +901,20 @@ async fn turn_start_additional_context_flows_to_model_input() -> Result<()> {
|
||||
let body = request
|
||||
.body_json::<Value>()
|
||||
.context("request body should be JSON")?;
|
||||
let input = body["input"].as_array().context("input array")?;
|
||||
let texts = input
|
||||
.iter()
|
||||
.filter_map(|item| item["content"].as_array())
|
||||
.flatten()
|
||||
.filter_map(|part| part["text"].as_str())
|
||||
.collect::<Vec<_>>();
|
||||
assert!(!texts.contains(&"rejected input"));
|
||||
assert!(!texts.contains(&"<application>rejected context</application>"));
|
||||
assert!(
|
||||
body.to_string()
|
||||
.contains("<external_custom_source>source value</external_custom_source>")
|
||||
);
|
||||
assert!(!body.to_string().contains("function_call_output"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ use codex_app_server_protocol::TurnSteerResponse;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS;
|
||||
use core_test_support::skip_if_remote;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use tempfile::TempDir;
|
||||
@@ -99,6 +100,41 @@ async fn turn_steer_requires_active_turn() -> Result<()> {
|
||||
);
|
||||
assert_eq!(event["event_params"]["rejection_reason"], "no_active_turn");
|
||||
|
||||
let malformed_request = mcp
|
||||
.send_turn_steer_request(TurnSteerParams {
|
||||
thread_id: thread.id,
|
||||
client_user_message_id: None,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "rejected input".to_owned(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
responsesapi_client_metadata: None,
|
||||
responsesapi_client_metadata_patch: None,
|
||||
additional_context: Some(HashMap::from([(
|
||||
"browser_info".to_owned(),
|
||||
AdditionalContextEntry {
|
||||
value: "rejected context".to_owned(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
name: Some("agent.browser".to_owned()),
|
||||
},
|
||||
)])),
|
||||
expected_turn_id: "turn-does-not-exist".to_owned(),
|
||||
})
|
||||
.await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(malformed_request)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(
|
||||
error.error,
|
||||
codex_app_server_protocol::JSONRPCErrorError {
|
||||
code: -32600,
|
||||
message: "additional context name must match [a-z][a-z0-9_]{0,63}".to_owned(),
|
||||
data: None,
|
||||
}
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -430,6 +466,7 @@ async fn turn_steer_rejects_context_only_input_without_merging_context() -> Resu
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
name: None,
|
||||
},
|
||||
)]));
|
||||
let steer_req = mcp
|
||||
|
||||
@@ -235,6 +235,12 @@ pub(super) async fn handle(
|
||||
submission_id: String,
|
||||
) -> CodexResult<TurnInputSubmission> {
|
||||
let request = request.into();
|
||||
request
|
||||
.request
|
||||
.additional_context
|
||||
.values()
|
||||
.try_for_each(AdditionalContextEntry::validate)
|
||||
.map_err(|message| CodexErr::InvalidRequest(message.to_owned()))?;
|
||||
let result = match mode {
|
||||
TurnInputMode::StartOrSteer => start_or_steer(session, request, submission_id).await,
|
||||
TurnInputMode::StartIfIdle => {
|
||||
|
||||
@@ -19,7 +19,11 @@ impl AdditionalContextStore {
|
||||
) -> Vec<ResponseItem> {
|
||||
let fragments = values
|
||||
.iter()
|
||||
.filter(|(key, value)| self.values.get(*key) != Some(*value))
|
||||
.filter(|(key, entry)| {
|
||||
self.values.get(*key).is_none_or(|previous| {
|
||||
previous.kind != entry.kind || previous.value != entry.value
|
||||
})
|
||||
})
|
||||
.map(|(key, entry)| match entry.kind {
|
||||
AdditionalContextKind::Untrusted => ContextualUserFragment::into(
|
||||
AdditionalContextUserFragment::new(key.clone(), entry.value.clone()),
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use anyhow::Result;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::protocol::AdditionalContextEntry;
|
||||
use codex_protocol::protocol::AdditionalContextKind;
|
||||
@@ -46,17 +47,17 @@ async fn additional_context_is_model_visible_but_not_a_user_message_item() -> Re
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"tab one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Application,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -181,17 +182,17 @@ async fn additional_context_trust_controls_message_role() -> Result<()> {
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"tab one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Application,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -222,8 +223,13 @@ async fn additional_context_trust_controls_message_role() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(None, None; "unchanged")]
|
||||
#[test_case::test_case(Some("browser"), Some("editor"); "name_only_change")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn additional_context_is_deduplicated_between_turns_while_retained() -> Result<()> {
|
||||
async fn additional_context_is_deduplicated_between_turns_while_retained(
|
||||
first_name: Option<&str>,
|
||||
second_name: Option<&str>,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -241,11 +247,12 @@ async fn additional_context_is_deduplicated_between_turns_while_retained() -> Re
|
||||
.with_config(|config| config.include_environment_context = false)
|
||||
.build(&server)
|
||||
.await?;
|
||||
let additional_context = BTreeMap::from([(
|
||||
let mut additional_context = BTreeMap::from([(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "same tab".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
name: first_name.map(str::to_owned),
|
||||
},
|
||||
)]);
|
||||
|
||||
@@ -263,6 +270,10 @@ async fn additional_context_is_deduplicated_between_turns_while_retained() -> Re
|
||||
})
|
||||
.await;
|
||||
|
||||
additional_context
|
||||
.get_mut("browser_info")
|
||||
.expect("browser context")
|
||||
.name = second_name.map(str::to_owned);
|
||||
test.codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
@@ -330,17 +341,17 @@ async fn additional_context_removes_one_value_while_adding_another() -> Result<(
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"tab one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -359,17 +370,17 @@ async fn additional_context_removes_one_value_while_adding_another() -> Result<(
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"terminal_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "pty one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"pty one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -388,24 +399,24 @@ async fn additional_context_removes_one_value_while_adding_another() -> Result<(
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"tab one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"terminal_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "pty one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"pty one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -481,17 +492,17 @@ async fn additional_context_values_are_truncated_before_model_input() -> Result<
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: long_automation_value.clone(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
long_automation_value.clone(),
|
||||
AdditionalContextKind::Application,
|
||||
),
|
||||
),
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: long_browser_value.clone(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
long_browser_value.clone(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
@@ -543,3 +554,53 @@ async fn additional_context_values_are_truncated_before_model_input() -> Result<
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn additional_context_rejects_invalid_names_before_admitting_input() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = start_mock_server().await;
|
||||
let request = mount_sse_once(&server, sse(vec![ev_completed("valid-turn")])).await;
|
||||
let test = test_codex()
|
||||
.with_config(|config| config.include_environment_context = false)
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
|
||||
for (kind, name) in [
|
||||
(AdditionalContextKind::Untrusted, String::new()),
|
||||
(AdditionalContextKind::Untrusted, "agent.browser".to_owned()),
|
||||
(AdditionalContextKind::Untrusted, "other.browser".to_owned()),
|
||||
(AdditionalContextKind::Untrusted, "1browser".to_owned()),
|
||||
(AdditionalContextKind::Untrusted, "Browser".to_owned()),
|
||||
(AdditionalContextKind::Untrusted, "é".to_owned()),
|
||||
(AdditionalContextKind::Untrusted, "a".repeat(65)),
|
||||
(AdditionalContextKind::Application, "browser".to_owned()),
|
||||
] {
|
||||
let result = test
|
||||
.codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "rejected input".to_owned(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_additional_context(BTreeMap::from([(
|
||||
"source".to_owned(),
|
||||
AdditionalContextEntry::new("rejected context".to_owned(), kind)
|
||||
.with_name(name),
|
||||
)])),
|
||||
)
|
||||
.await;
|
||||
assert!(matches!(
|
||||
result
|
||||
.expect_err("invalid context must be rejected")
|
||||
.details(),
|
||||
CodexErrorDetails::InvalidRequest(_)
|
||||
));
|
||||
}
|
||||
assert!(request.requests().is_empty());
|
||||
test.submit_text_turn("valid input").await?;
|
||||
assert_eq!(
|
||||
request.single_request().message_input_texts("user"),
|
||||
vec!["valid input"]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -125,17 +125,17 @@ async fn first_request_item_types_roles_and_content_annotations() -> Result<()>
|
||||
.with_additional_context(BTreeMap::from([
|
||||
(
|
||||
"browser_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "tab one".to_string(),
|
||||
kind: AdditionalContextKind::Untrusted,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"tab one".to_string(),
|
||||
AdditionalContextKind::Untrusted,
|
||||
),
|
||||
),
|
||||
(
|
||||
"automation_info".to_string(),
|
||||
AdditionalContextEntry {
|
||||
value: "run one".to_string(),
|
||||
kind: AdditionalContextKind::Application,
|
||||
},
|
||||
AdditionalContextEntry::new(
|
||||
"run one".to_string(),
|
||||
AdditionalContextKind::Application,
|
||||
),
|
||||
),
|
||||
])),
|
||||
)
|
||||
|
||||
@@ -646,6 +646,45 @@ pub enum AdditionalContextKind {
|
||||
pub struct AdditionalContextEntry {
|
||||
pub value: String,
|
||||
pub kind: AdditionalContextKind,
|
||||
/// Optional name for untrusted context, used to label its tool output.
|
||||
pub name: Option<String>,
|
||||
}
|
||||
|
||||
impl AdditionalContextEntry {
|
||||
/// Creates an unnamed context entry.
|
||||
pub fn new(value: String, kind: AdditionalContextKind) -> Self {
|
||||
Self {
|
||||
value,
|
||||
kind,
|
||||
name: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Sets the name for untrusted context.
|
||||
pub fn with_name(mut self, name: impl Into<String>) -> Self {
|
||||
self.name = Some(name.into());
|
||||
self
|
||||
}
|
||||
|
||||
/// Validates the optional untrusted-context name before input admission.
|
||||
pub fn validate(&self) -> Result<(), &'static str> {
|
||||
let Some(name) = &self.name else {
|
||||
return Ok(());
|
||||
};
|
||||
if self.kind == AdditionalContextKind::Application {
|
||||
return Err("additional context name is only supported for untrusted context");
|
||||
}
|
||||
if name.is_empty()
|
||||
|| name.len() > 64
|
||||
|| !name.as_bytes()[0].is_ascii_lowercase()
|
||||
|| !name
|
||||
.bytes()
|
||||
.all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_')
|
||||
{
|
||||
return Err("additional context name must match [a-z][a-z0-9_]{0,63}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// Submission operation
|
||||
|
||||
@@ -6901,6 +6901,10 @@ class AdditionalContextEntry(BaseModel):
|
||||
populate_by_name=True,
|
||||
)
|
||||
kind: AdditionalContextKind
|
||||
name: Annotated[
|
||||
str | None,
|
||||
Field(description="Optional name for untrusted context, used to label its tool output."),
|
||||
] = None
|
||||
value: str
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user