mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Persist complete TurnContextItem state via canonical conversion (#11656)
## Summary
This PR delivers the first small, shippable step toward model-visible
state diffing by making
`TurnContextItem` more complete and standardizing how it is built.
Specifically, it:
- Adds persisted network context to `TurnContextItem`.
- Introduces a single canonical `TurnContext -> TurnContextItem`
conversion path.
- Routes existing rollout write sites through that canonical conversion
helper.
No context injection/diff behavior changes are included in this PR.
## Why this change
The design goal is to make `TurnContextItem` the canonical source of
truth for context-diff
decisions.
Before this PR:
- `TurnContextItem` did not include all TurnContext-derived environment
inputs needed for v1
completeness.
- Construction was duplicated at multiple write sites.
This PR addresses both with a minimal, reviewable change.
## Changes
### 1) Extend `TurnContextItem` with network state
- Added `TurnContextNetworkItem { allowed_domains, denied_domains }`.
- Added `network: Option<TurnContextNetworkItem>` to `TurnContextItem`.
- Kept backward compatibility by making the new field optional and
skipped when absent.
Files:
- `codex-rs/protocol/src/protocol.rs`
### 2) Canonical conversion helper
- Added `TurnContext::to_turn_context_item(collaboration_mode)` in core.
- Added internal helper to derive network fields from
`config_layer_stack.requirements().network`.
Files:
- `codex-rs/core/src/codex.rs`
### 3) Use canonical conversion at rollout write sites
- Replaced ad hoc `TurnContextItem { ... }` construction with
`to_turn_context_item(...)` in:
- sampling request path
- compaction path
Files:
- `codex-rs/core/src/codex.rs`
- `codex-rs/core/src/compact.rs`
### 4) Update fixtures/tests for new optional field
- Updated existing `TurnContextItem` literals in tests to include
`network: None`.
- Added protocol tests for:
- deserializing old payloads with no `network`
- serializing when `network` is present
Files:
- `codex-rs/core/tests/suite/resume_warning.rs`
- No replay/diff logic changes.
- Persisted rollout `TurnContextItem` now carries additional network
context when available.
- Older rollout lines without `network` remain readable.
This commit is contained in:
committed by
GitHub
Unverified
parent
46b2da35d5
commit
f24669d444
@@ -564,6 +564,7 @@ fn append_rollout_turn_context(path: &Path, timestamp: &str, model: &str) -> std
|
||||
cwd: PathBuf::from("/"),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
network: None,
|
||||
model: model.to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
|
||||
+100
-15
@@ -73,6 +73,7 @@ use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::TurnContextNetworkItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
@@ -655,6 +656,41 @@ impl TurnContext {
|
||||
.unwrap_or(compact::SUMMARIZATION_PROMPT)
|
||||
}
|
||||
|
||||
pub(crate) fn to_turn_context_item(
|
||||
&self,
|
||||
collaboration_mode: CollaborationMode,
|
||||
) -> TurnContextItem {
|
||||
TurnContextItem {
|
||||
turn_id: Some(self.sub_id.clone()),
|
||||
cwd: self.cwd.clone(),
|
||||
approval_policy: self.approval_policy,
|
||||
sandbox_policy: self.sandbox_policy.clone(),
|
||||
network: self.turn_context_network_item(),
|
||||
model: self.model_info.slug.clone(),
|
||||
personality: self.personality,
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
effort: self.reasoning_effort,
|
||||
summary: self.reasoning_summary,
|
||||
user_instructions: self.user_instructions.clone(),
|
||||
developer_instructions: self.developer_instructions.clone(),
|
||||
final_output_json_schema: self.final_output_json_schema.clone(),
|
||||
truncation_policy: Some(self.truncation_policy.into()),
|
||||
}
|
||||
}
|
||||
|
||||
fn turn_context_network_item(&self) -> Option<TurnContextNetworkItem> {
|
||||
let network = self
|
||||
.config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.network
|
||||
.as_ref()?;
|
||||
Some(TurnContextNetworkItem {
|
||||
allowed_domains: network.allowed_domains.clone().unwrap_or_default(),
|
||||
denied_domains: network.denied_domains.clone().unwrap_or_default(),
|
||||
})
|
||||
}
|
||||
|
||||
async fn build_turn_metadata_header(&self) -> Option<String> {
|
||||
let sandbox = sandbox_tag(&self.sandbox_policy, self.windows_sandbox_level);
|
||||
self.turn_metadata_header
|
||||
@@ -5236,21 +5272,8 @@ async fn try_run_sampling_request(
|
||||
cancellation_token: CancellationToken,
|
||||
) -> CodexResult<SamplingRequestResult> {
|
||||
let collaboration_mode = sess.current_collaboration_mode().await;
|
||||
let rollout_item = RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
personality: turn_context.personality,
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: turn_context.user_instructions.clone(),
|
||||
developer_instructions: turn_context.developer_instructions.clone(),
|
||||
final_output_json_schema: turn_context.final_output_json_schema.clone(),
|
||||
truncation_policy: Some(turn_context.truncation_policy.into()),
|
||||
});
|
||||
let rollout_item =
|
||||
RolloutItem::TurnContext(turn_context.to_turn_context_item(collaboration_mode));
|
||||
|
||||
feedback_tags!(
|
||||
model = turn_context.model_info.slug.clone(),
|
||||
@@ -5550,6 +5573,11 @@ mod tests {
|
||||
use crate::CodexAuth;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::test_config;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigLayerStackOrdering;
|
||||
use crate::config_loader::NetworkConstraints;
|
||||
use crate::config_loader::RequirementSource;
|
||||
use crate::config_loader::Sourced;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::mcp_connection_manager::ToolInfo;
|
||||
@@ -6024,6 +6052,7 @@ mod tests {
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: turn_context.personality,
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
@@ -6242,6 +6271,7 @@ mod tests {
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: turn_context.personality,
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
@@ -7232,6 +7262,61 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_emits_environment_item_for_network_changes() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
let previous_context = Arc::new(previous_context);
|
||||
let mut current_context = previous_context
|
||||
.with_model(
|
||||
previous_context.model_info.slug.clone(),
|
||||
&session.services.models_manager,
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut config = (*current_context.config).clone();
|
||||
let mut requirements = config.config_layer_stack.requirements().clone();
|
||||
requirements.network = Some(Sourced::new(
|
||||
NetworkConstraints {
|
||||
allowed_domains: Some(vec!["api.example.com".to_string()]),
|
||||
denied_domains: Some(vec!["blocked.example.com".to_string()]),
|
||||
..Default::default()
|
||||
},
|
||||
RequirementSource::CloudRequirements,
|
||||
));
|
||||
let layers = config
|
||||
.config_layer_stack
|
||||
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
|
||||
.into_iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
config.config_layer_stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
requirements,
|
||||
config.config_layer_stack.requirements_toml().clone(),
|
||||
)
|
||||
.expect("rebuild config layer stack with network requirements");
|
||||
current_context.config = Arc::new(config);
|
||||
|
||||
let update_items =
|
||||
session.build_settings_update_items(Some(&previous_context), None, ¤t_context);
|
||||
|
||||
let environment_update = update_items
|
||||
.iter()
|
||||
.find_map(|item| match item {
|
||||
ResponseItem::Message { role, content, .. } if role == "user" => {
|
||||
let [ContentItem::InputText { text }] = content.as_slice() else {
|
||||
return None;
|
||||
};
|
||||
text.contains("<environment_context>").then_some(text)
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.expect("environment update item should be emitted");
|
||||
assert!(environment_update.contains("<network enabled=\"true\">"));
|
||||
assert!(environment_update.contains("<allowed>api.example.com</allowed>"));
|
||||
assert!(environment_update.contains("<denied>blocked.example.com</denied>"));
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct NeverEndingTask {
|
||||
kind: TaskKind,
|
||||
|
||||
@@ -11,7 +11,6 @@ use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::protocol::CompactedItem;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::TurnContextItem;
|
||||
use crate::protocol::TurnStartedEvent;
|
||||
use crate::protocol::WarningEvent;
|
||||
use crate::truncate::TruncationPolicy;
|
||||
@@ -95,21 +94,8 @@ async fn run_compact_task_inner(
|
||||
// duplicating model settings on TurnContext, but an Op after turn start could update the
|
||||
// session config before this write occurs.
|
||||
let collaboration_mode = sess.current_collaboration_mode().await;
|
||||
let rollout_item = RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
approval_policy: turn_context.approval_policy,
|
||||
sandbox_policy: turn_context.sandbox_policy.clone(),
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
personality: turn_context.personality,
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: turn_context.user_instructions.clone(),
|
||||
developer_instructions: turn_context.developer_instructions.clone(),
|
||||
final_output_json_schema: turn_context.final_output_json_schema.clone(),
|
||||
truncation_policy: Some(turn_context.truncation_policy.into()),
|
||||
});
|
||||
let rollout_item =
|
||||
RolloutItem::TurnContext(turn_context.to_turn_context_item(collaboration_mode));
|
||||
sess.persist_rollout_items(&[rollout_item]).await;
|
||||
|
||||
loop {
|
||||
|
||||
@@ -24,6 +24,7 @@ fn resume_history(
|
||||
cwd: config.cwd.clone(),
|
||||
approval_policy: config.permissions.approval_policy.value(),
|
||||
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
|
||||
network: None,
|
||||
model: previous_model.to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
|
||||
@@ -1938,6 +1938,12 @@ impl From<CompactedItem> for ResponseItem {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, TS)]
|
||||
pub struct TurnContextNetworkItem {
|
||||
pub allowed_domains: Vec<String>,
|
||||
pub denied_domains: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)]
|
||||
pub struct TurnContextItem {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -1945,6 +1951,8 @@ pub struct TurnContextItem {
|
||||
pub cwd: PathBuf,
|
||||
pub approval_policy: AskForApproval,
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub network: Option<TurnContextNetworkItem>,
|
||||
pub model: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub personality: Option<Personality>,
|
||||
@@ -2981,6 +2989,53 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_item_deserializes_without_network() -> Result<()> {
|
||||
let item: TurnContextItem = serde_json::from_value(json!({
|
||||
"cwd": "/tmp",
|
||||
"approval_policy": "never",
|
||||
"sandbox_policy": { "type": "danger-full-access" },
|
||||
"model": "gpt-5",
|
||||
"summary": "auto",
|
||||
}))?;
|
||||
|
||||
assert_eq!(item.network, None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_item_serializes_network_when_present() -> Result<()> {
|
||||
let item = TurnContextItem {
|
||||
turn_id: None,
|
||||
cwd: PathBuf::from("/tmp"),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
network: Some(TurnContextNetworkItem {
|
||||
allowed_domains: vec!["api.example.com".to_string()],
|
||||
denied_domains: vec!["blocked.example.com".to_string()],
|
||||
}),
|
||||
model: "gpt-5".to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
effort: None,
|
||||
summary: ReasoningSummaryConfig::Auto,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: None,
|
||||
};
|
||||
|
||||
let value = serde_json::to_value(item)?;
|
||||
assert_eq!(
|
||||
value["network"],
|
||||
json!({
|
||||
"allowed_domains": ["api.example.com"],
|
||||
"denied_domains": ["blocked.example.com"],
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Serialize Event to verify that its JSON representation has the expected
|
||||
/// amount of nesting.
|
||||
#[test]
|
||||
|
||||
@@ -1008,6 +1008,7 @@ mod tests {
|
||||
cwd,
|
||||
approval_policy: config.permissions.approval_policy.value(),
|
||||
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
|
||||
network: None,
|
||||
model,
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
|
||||
Reference in New Issue
Block a user