mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] core: restore absolute turn context cwd (#28629)
## Why #28152 jumped the gun on moving the rollout format to store URIs, and would likely break compat with some features that don't go through the same types as the core logic. ## What Make `TurnContextItem.cwd` an `AbsolutePathBuf` again, remove test added for `PathUri` serialization in rollouts. Also drops a bunch of error paths that are no longer needed.
This commit is contained in:
committed by
GitHub
Unverified
parent
69bc0645ac
commit
3d02c443bc
@@ -458,7 +458,7 @@ impl CodexThread {
|
||||
self.codex
|
||||
.session
|
||||
.record_context_updates_and_set_reference_context_item(turn_context.as_ref())
|
||||
.await?;
|
||||
.await;
|
||||
}
|
||||
self.codex
|
||||
.session
|
||||
|
||||
@@ -380,12 +380,12 @@ impl EnvironmentContext {
|
||||
pub(crate) fn diff_from_turn_context_item(
|
||||
before: &TurnContextItem,
|
||||
after: &EnvironmentContext,
|
||||
) -> std::io::Result<Self> {
|
||||
) -> Self {
|
||||
let before_network = Self::network_from_turn_context_item(before);
|
||||
let before_filesystem = Self::filesystem_from_turn_context_item(before)?;
|
||||
let before_filesystem = Self::filesystem_from_turn_context_item(before);
|
||||
let environments = match &after.environments {
|
||||
EnvironmentContextEnvironments::Single(environment) => {
|
||||
if before.cwd != environment.cwd {
|
||||
if PathUri::from_abs_path(&before.cwd) != environment.cwd {
|
||||
EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy(
|
||||
environment.cwd.clone(),
|
||||
environment.shell.clone(),
|
||||
@@ -409,14 +409,14 @@ impl EnvironmentContext {
|
||||
} else {
|
||||
before_filesystem
|
||||
};
|
||||
Ok(EnvironmentContext::new_with_environments(
|
||||
EnvironmentContext::new_with_environments(
|
||||
environments,
|
||||
after.current_date.clone(),
|
||||
after.timezone.clone(),
|
||||
network,
|
||||
filesystem,
|
||||
/*subagents*/ None,
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
|
||||
@@ -440,18 +440,18 @@ impl EnvironmentContext {
|
||||
pub(crate) fn from_turn_context_item(
|
||||
turn_context_item: &TurnContextItem,
|
||||
shell: String,
|
||||
) -> std::io::Result<Self> {
|
||||
Ok(Self::new_with_environments(
|
||||
) -> Self {
|
||||
Self::new_with_environments(
|
||||
EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy(
|
||||
turn_context_item.cwd.clone(),
|
||||
PathUri::from_abs_path(&turn_context_item.cwd),
|
||||
shell,
|
||||
)]),
|
||||
turn_context_item.current_date.clone(),
|
||||
turn_context_item.timezone.clone(),
|
||||
Self::network_from_turn_context_item(turn_context_item),
|
||||
Self::filesystem_from_turn_context_item(turn_context_item)?,
|
||||
Self::filesystem_from_turn_context_item(turn_context_item),
|
||||
/*subagents*/ None,
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn with_subagents(mut self, subagents: String) -> Self {
|
||||
@@ -498,11 +498,11 @@ impl EnvironmentContext {
|
||||
|
||||
fn filesystem_from_turn_context_item(
|
||||
turn_context_item: &TurnContextItem,
|
||||
) -> std::io::Result<Option<FileSystemContext>> {
|
||||
Ok(Some(FileSystemContext::from_permission_profile(
|
||||
&turn_context_item.permission_profile()?,
|
||||
) -> Option<FileSystemContext> {
|
||||
Some(FileSystemContext::from_permission_profile(
|
||||
&turn_context_item.permission_profile(),
|
||||
&workspace_roots_from_turn_context_item(turn_context_item),
|
||||
)))
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -513,12 +513,7 @@ fn workspace_roots_from_turn_context_item(
|
||||
return workspace_roots.clone();
|
||||
}
|
||||
|
||||
// Older rollout items did not persist workspace roots. Fall back to the
|
||||
// legacy cwd binding only when reconstructing that historical context.
|
||||
match turn_context_item.cwd.to_abs_path() {
|
||||
Ok(cwd) => vec![cwd],
|
||||
Err(_) => Vec::new(),
|
||||
}
|
||||
vec![turn_context_item.cwd.clone()]
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for EnvironmentContext {
|
||||
|
||||
@@ -190,7 +190,7 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
|
||||
let repo_private = repo.join("private");
|
||||
let item = TurnContextItem {
|
||||
turn_id: None,
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/not-the-workspace")),
|
||||
cwd: test_abs_path("/not-the-workspace"),
|
||||
workspace_roots: Some(vec![repo.clone(), other_repo.clone()]),
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
@@ -209,9 +209,7 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
|
||||
let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name())
|
||||
.expect("turn context should hydrate")
|
||||
.render();
|
||||
let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()).render();
|
||||
|
||||
assert!(
|
||||
context.contains(&format!(
|
||||
|
||||
@@ -21,9 +21,9 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::truncate_text;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use image::ImageBuffer;
|
||||
use image::ImageFormat;
|
||||
use image::Luma;
|
||||
@@ -127,7 +127,7 @@ fn developer_msg_with_fragments(texts: &[&str]) -> ResponseItem {
|
||||
fn reference_context_item() -> TurnContextItem {
|
||||
TurnContextItem {
|
||||
turn_id: Some("reference-turn".to_string()),
|
||||
cwd: PathUri::from_path(
|
||||
cwd: AbsolutePathBuf::try_from(
|
||||
std::env::current_dir()
|
||||
.expect("current directory")
|
||||
.join("reference-cwd"),
|
||||
|
||||
@@ -22,44 +22,40 @@ fn build_environment_update_item(
|
||||
previous: Option<&TurnContextItem>,
|
||||
next: &TurnContext,
|
||||
shell: &Shell,
|
||||
) -> std::io::Result<Option<ResponseItem>> {
|
||||
) -> Option<ResponseItem> {
|
||||
if !next.config.include_environment_context {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
|
||||
let Some(prev) = previous else {
|
||||
return Ok(None);
|
||||
};
|
||||
let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string())?;
|
||||
let prev = previous?;
|
||||
let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string());
|
||||
let next_context = EnvironmentContext::from_turn_context(next, shell);
|
||||
if prev_context.equals_except_shell(&next_context) {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
|
||||
Ok(Some(ContextualUserFragment::into(
|
||||
EnvironmentContext::diff_from_turn_context_item(prev, &next_context)?,
|
||||
)))
|
||||
Some(ContextualUserFragment::into(
|
||||
EnvironmentContext::diff_from_turn_context_item(prev, &next_context),
|
||||
))
|
||||
}
|
||||
|
||||
fn build_permissions_update_item(
|
||||
previous: Option<&TurnContextItem>,
|
||||
next: &TurnContext,
|
||||
exec_policy: &Policy,
|
||||
) -> std::io::Result<Option<String>> {
|
||||
) -> Option<String> {
|
||||
if !next.config.include_permissions_instructions {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
|
||||
let Some(prev) = previous else {
|
||||
return Ok(None);
|
||||
};
|
||||
if prev.permission_profile()? == next.permission_profile()
|
||||
let prev = previous?;
|
||||
if prev.permission_profile() == next.permission_profile()
|
||||
&& prev.approval_policy == next.approval_policy.value()
|
||||
{
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
|
||||
Ok(Some(
|
||||
Some(
|
||||
PermissionsInstructions::from_permission_profile(
|
||||
&next.permission_profile,
|
||||
next.approval_policy.value(),
|
||||
@@ -71,7 +67,7 @@ fn build_permissions_update_item(
|
||||
next.features.enabled(Feature::RequestPermissionsTool),
|
||||
)
|
||||
.render(),
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
fn build_collaboration_mode_update_item(
|
||||
@@ -218,17 +214,17 @@ pub(crate) fn build_settings_update_items(
|
||||
shell: &Shell,
|
||||
exec_policy: &Policy,
|
||||
personality_feature_enabled: bool,
|
||||
) -> std::io::Result<Vec<ResponseItem>> {
|
||||
) -> Vec<ResponseItem> {
|
||||
// TODO(ccunningham): build_settings_update_items still does not cover every
|
||||
// model-visible item emitted by build_initial_context. Persist the remaining
|
||||
// inputs or add explicit replay events so fork/resume can diff everything
|
||||
// deterministically.
|
||||
let contextual_user_message = build_environment_update_item(previous, next, shell)?;
|
||||
let contextual_user_message = build_environment_update_item(previous, next, shell);
|
||||
let developer_update_sections = [
|
||||
// Keep model-switch instructions first so model-specific guidance is read before
|
||||
// any other context diffs on this turn.
|
||||
build_model_instructions_update_item(previous_turn_settings, next),
|
||||
build_permissions_update_item(previous, next, exec_policy)?,
|
||||
build_permissions_update_item(previous, next, exec_policy),
|
||||
build_collaboration_mode_update_item(previous, next),
|
||||
build_realtime_update_item(previous, previous_turn_settings, next),
|
||||
build_personality_update_item(previous, next, personality_feature_enabled),
|
||||
@@ -244,5 +240,5 @@ pub(crate) fn build_settings_update_items(
|
||||
if let Some(contextual_user_message) = contextual_user_message {
|
||||
items.push(contextual_user_message);
|
||||
}
|
||||
Ok(items)
|
||||
items
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@ pub(crate) async fn build_prompt_input_from_session(
|
||||
) -> CodexResult<Vec<ResponseItem>> {
|
||||
let turn_context = sess.new_default_turn().await;
|
||||
sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref())
|
||||
.await?;
|
||||
.await;
|
||||
|
||||
if !input.is_empty() {
|
||||
let response_item = sess.response_item_from_user_input(turn_context.as_ref(), input);
|
||||
|
||||
@@ -1621,7 +1621,7 @@ impl Session {
|
||||
&self,
|
||||
reference_context_item: Option<&TurnContextItem>,
|
||||
current_context: &TurnContext,
|
||||
) -> CodexResult<Vec<ResponseItem>> {
|
||||
) -> Vec<ResponseItem> {
|
||||
// TODO: Make context updates a pure diff of persisted previous/current TurnContextItem
|
||||
// state so replay/backtracking is deterministic. Runtime inputs that affect model-visible
|
||||
// context (shell, exec policy, feature gates, previous-turn bridge) should be persisted
|
||||
@@ -1632,15 +1632,13 @@ impl Session {
|
||||
};
|
||||
let shell = self.user_shell();
|
||||
let exec_policy = self.services.exec_policy.current();
|
||||
Ok(
|
||||
crate::context_manager::updates::build_settings_update_items(
|
||||
reference_context_item,
|
||||
previous_turn_settings.as_ref(),
|
||||
current_context,
|
||||
shell.as_ref(),
|
||||
exec_policy.as_ref(),
|
||||
self.features.enabled(Feature::Personality),
|
||||
)?,
|
||||
crate::context_manager::updates::build_settings_update_items(
|
||||
reference_context_item,
|
||||
previous_turn_settings.as_ref(),
|
||||
current_context,
|
||||
shell.as_ref(),
|
||||
exec_policy.as_ref(),
|
||||
self.features.enabled(Feature::Personality),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -3188,7 +3186,7 @@ impl Session {
|
||||
pub(crate) async fn record_context_updates_and_set_reference_context_item(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
) -> CodexResult<()> {
|
||||
) {
|
||||
let reference_context_item = {
|
||||
let state = self.state.lock().await;
|
||||
state.reference_context_item()
|
||||
@@ -3199,7 +3197,7 @@ impl Session {
|
||||
} else {
|
||||
// Steady-state path: append only context diffs to minimize token overhead.
|
||||
self.build_settings_update_items(reference_context_item.as_ref(), turn_context)
|
||||
.await?
|
||||
.await
|
||||
};
|
||||
let turn_context_item = turn_context.to_turn_context_item();
|
||||
if !context_items.is_empty() {
|
||||
@@ -3215,7 +3213,6 @@ impl Session {
|
||||
// context items. This keeps later runtime diffing aligned with the current turn state.
|
||||
let mut state = self.state.lock().await;
|
||||
state.set_reference_context_item(Some(turn_context_item));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn update_token_usage_info(
|
||||
|
||||
@@ -9,7 +9,6 @@ use codex_protocol::protocol::CompactedItem;
|
||||
use codex_protocol::protocol::InitialHistory;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use codex_protocol::protocol::ResumedHistory;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -89,7 +88,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -129,7 +128,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
|
||||
let mut previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -990,7 +989,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -1072,7 +1071,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
serde_json::to_value(Some(TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -1102,7 +1101,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -1224,7 +1223,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
|
||||
let current_context_item = TurnContextItem {
|
||||
turn_id: Some(current_turn_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -1344,7 +1343,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -1507,7 +1506,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
|
||||
@@ -1874,8 +1874,7 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only()
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
expected.extend(initial_context);
|
||||
let history_after_seed = session.clone_history().await;
|
||||
@@ -1883,8 +1882,7 @@ async fn resumed_history_injects_initial_context_on_first_context_update_only()
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
let history_after_second_seed = session.clone_history().await;
|
||||
assert_eq!(
|
||||
history_after_seed.raw_items(),
|
||||
@@ -2675,7 +2673,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
#[allow(deprecated)]
|
||||
cwd: PathUri::from_abs_path(&turn_context.cwd),
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: turn_context.current_date.clone(),
|
||||
timezone: turn_context.timezone.clone(),
|
||||
@@ -7292,8 +7290,7 @@ async fn build_settings_update_items_emits_environment_item_for_network_changes(
|
||||
let reference_context_item = previous_context.to_turn_context_item();
|
||||
let update_items = session
|
||||
.build_settings_update_items(Some(&reference_context_item), ¤t_context)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let environment_update = user_input_texts(&update_items)
|
||||
.into_iter()
|
||||
@@ -7363,8 +7360,7 @@ async fn build_settings_update_items_emits_environment_item_for_time_changes() {
|
||||
let reference_context_item = previous_context.to_turn_context_item();
|
||||
let update_items = session
|
||||
.build_settings_update_items(Some(&reference_context_item), ¤t_context)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let environment_update = user_input_texts(&update_items)
|
||||
.into_iter()
|
||||
@@ -7392,8 +7388,7 @@ async fn build_settings_update_items_omits_environment_item_when_disabled() {
|
||||
let reference_context_item = previous_context.to_turn_context_item();
|
||||
let update_items = session
|
||||
.build_settings_update_items(Some(&reference_context_item), ¤t_context)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let user_texts = user_input_texts(&update_items);
|
||||
assert!(
|
||||
@@ -7421,8 +7416,7 @@ async fn build_settings_update_items_emits_realtime_start_when_session_becomes_l
|
||||
Some(&previous_context.to_turn_context_item()),
|
||||
¤t_context,
|
||||
)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let developer_texts = developer_input_texts(&update_items);
|
||||
assert!(
|
||||
@@ -7450,8 +7444,7 @@ async fn build_settings_update_items_emits_realtime_end_when_session_stops_being
|
||||
Some(&previous_context.to_turn_context_item()),
|
||||
¤t_context,
|
||||
)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let developer_texts = developer_input_texts(&update_items);
|
||||
assert!(
|
||||
@@ -7485,8 +7478,7 @@ async fn build_settings_update_items_uses_previous_turn_settings_for_realtime_en
|
||||
.await;
|
||||
let update_items = session
|
||||
.build_settings_update_items(Some(&previous_context_item), ¤t_context)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
|
||||
let developer_texts = developer_input_texts(&update_items);
|
||||
assert!(
|
||||
@@ -8139,18 +8131,20 @@ async fn turn_context_item_uses_turn_context_comp_hash_snapshot() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_context_item_stores_primary_environment_cwd_uri() {
|
||||
async fn turn_context_item_stores_local_cwd() {
|
||||
let (_session, mut turn_context) = make_session_and_context().await;
|
||||
let environment = turn_context.environments.turn_environments[0].clone();
|
||||
let cwd = PathUri::parse("file:///C:/windows").expect("Windows cwd URI");
|
||||
turn_context.environments.turn_environments[0] = TurnEnvironment::new(
|
||||
"remote".to_string(),
|
||||
environment.environment,
|
||||
cwd.clone(),
|
||||
cwd,
|
||||
environment.shell,
|
||||
);
|
||||
|
||||
assert_eq!(turn_context.to_turn_context_item().cwd, cwd);
|
||||
#[allow(deprecated)]
|
||||
let local_cwd = turn_context.cwd.clone();
|
||||
assert_eq!(turn_context.to_turn_context_item().cwd, local_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8194,8 +8188,7 @@ async fn record_context_updates_and_set_reference_context_item_injects_full_cont
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
let history = session.clone_history().await;
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
assert_eq!(history.raw_items().to_vec(), initial_context);
|
||||
@@ -8226,8 +8219,7 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
||||
.await;
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.set_reference_context_item(/*item*/ None);
|
||||
@@ -8241,8 +8233,7 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
|
||||
let history = session.clone_history().await;
|
||||
let mut expected_history = vec![compacted_summary];
|
||||
@@ -8272,14 +8263,12 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline
|
||||
|
||||
let update_items = session
|
||||
.build_settings_update_items(Some(&previous_context_item), &turn_context)
|
||||
.await
|
||||
.expect("settings updates should hydrate");
|
||||
.await;
|
||||
assert_eq!(update_items, Vec::new());
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
session.clone_history().await.raw_items().to_vec(),
|
||||
@@ -8326,8 +8315,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_split_fi
|
||||
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
session.ensure_rollout_materialized().await;
|
||||
session.flush_rollout().await.expect("rollout should flush");
|
||||
|
||||
@@ -8411,8 +8399,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei
|
||||
.await;
|
||||
session
|
||||
.record_context_updates_and_set_reference_context_item(&turn_context)
|
||||
.await
|
||||
.expect("context updates should hydrate");
|
||||
.await;
|
||||
session.ensure_rollout_materialized().await;
|
||||
session.flush_rollout().await.expect("rollout should flush");
|
||||
|
||||
|
||||
@@ -159,16 +159,8 @@ pub(crate) async fn run_turn(
|
||||
return None;
|
||||
}
|
||||
|
||||
if let Err(err) = sess
|
||||
.record_context_updates_and_set_reference_context_item(turn_context.as_ref())
|
||||
.await
|
||||
{
|
||||
let error = err.to_codex_protocol_error();
|
||||
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
|
||||
.await;
|
||||
error!(%err, "failed to hydrate persisted turn context");
|
||||
return None;
|
||||
}
|
||||
sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref())
|
||||
.await;
|
||||
|
||||
let (injection_items, explicitly_enabled_connectors) =
|
||||
build_skills_and_plugins(&sess, turn_context.as_ref(), &input, &cancellation_token).await?;
|
||||
|
||||
@@ -396,14 +396,8 @@ impl TurnContext {
|
||||
|
||||
pub(crate) fn to_turn_context_item(&self) -> TurnContextItem {
|
||||
let workspace_roots = self.config.effective_workspace_roots();
|
||||
let cwd = self
|
||||
.environments
|
||||
.primary()
|
||||
.map(|environment| environment.cwd().clone())
|
||||
.unwrap_or_else(|| {
|
||||
#[allow(deprecated)]
|
||||
PathUri::from_abs_path(&self.cwd)
|
||||
});
|
||||
#[allow(deprecated)]
|
||||
let cwd = self.cwd.clone();
|
||||
TurnContextItem {
|
||||
turn_id: Some(self.sub_id.clone()),
|
||||
cwd,
|
||||
|
||||
@@ -14,7 +14,6 @@ use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core::time::Duration;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::wait_for_event;
|
||||
@@ -28,7 +27,7 @@ fn resume_history(
|
||||
let turn_id = "resume-warning-seed-turn".to_string();
|
||||
let turn_ctx = TurnContextItem {
|
||||
turn_id: Some(turn_id.clone()),
|
||||
cwd: PathUri::from_abs_path(&config.cwd),
|
||||
cwd: config.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
|
||||
@@ -1423,11 +1423,7 @@ async fn parse_latest_turn_context_cwd(path: &Path) -> Option<PathBuf> {
|
||||
continue;
|
||||
};
|
||||
if let RolloutItem::TurnContext(item) = rollout_line.item {
|
||||
return item
|
||||
.cwd
|
||||
.to_abs_path()
|
||||
.ok()
|
||||
.map(AbsolutePathBuf::into_path_buf);
|
||||
return Some(item.cwd.into_path_buf());
|
||||
}
|
||||
}
|
||||
None
|
||||
|
||||
@@ -2988,7 +2988,7 @@ pub struct TurnContextNetworkItem {
|
||||
pub struct TurnContextItem {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub turn_id: Option<String>,
|
||||
pub cwd: PathUri,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
/// Effective workspace roots used to materialize symbolic
|
||||
/// `:workspace_roots` filesystem permissions in `permission_profile`.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -3026,35 +3026,21 @@ pub struct TurnContextItem {
|
||||
}
|
||||
|
||||
impl TurnContextItem {
|
||||
pub fn permission_profile(&self) -> std::io::Result<PermissionProfile> {
|
||||
if let Some(permission_profile) = self.permission_profile.clone() {
|
||||
return Ok(permission_profile);
|
||||
}
|
||||
let file_system_sandbox_policy = match self.file_system_sandbox_policy.clone() {
|
||||
Some(file_system_sandbox_policy) => file_system_sandbox_policy,
|
||||
None => {
|
||||
let cwd = self.cwd.to_abs_path().map_err(|err| {
|
||||
std::io::Error::new(
|
||||
err.kind(),
|
||||
format!(
|
||||
"cannot hydrate legacy permission profile for cwd {}: {err}",
|
||||
self.cwd
|
||||
),
|
||||
pub fn permission_profile(&self) -> PermissionProfile {
|
||||
self.permission_profile.clone().unwrap_or_else(|| {
|
||||
let file_system_sandbox_policy =
|
||||
self.file_system_sandbox_policy.clone().unwrap_or_else(|| {
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&self.sandbox_policy,
|
||||
self.cwd.as_path(),
|
||||
)
|
||||
})?;
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
&self.sandbox_policy,
|
||||
cwd.as_path(),
|
||||
)
|
||||
}
|
||||
};
|
||||
Ok(
|
||||
});
|
||||
PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
SandboxEnforcement::from_legacy_sandbox_policy(&self.sandbox_policy),
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::from(&self.sandbox_policy),
|
||||
),
|
||||
)
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5315,50 +5301,21 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_item_accepts_legacy_cwd_and_serializes_path_uri() -> Result<()> {
|
||||
let legacy_cwd = test_path_buf("/tmp");
|
||||
fn turn_context_item_deserializes_without_network() -> Result<()> {
|
||||
let item: TurnContextItem = serde_json::from_value(json!({
|
||||
"cwd": legacy_cwd,
|
||||
"cwd": test_path_buf("/tmp"),
|
||||
"approval_policy": "never",
|
||||
"sandbox_policy": { "type": "danger-full-access" },
|
||||
"model": "gpt-5",
|
||||
"summary": "auto",
|
||||
}))?;
|
||||
|
||||
let expected_cwd = PathUri::from_path(legacy_cwd)?;
|
||||
assert_eq!(item.cwd, expected_cwd);
|
||||
assert_eq!(
|
||||
serde_json::to_value(&item)?["cwd"],
|
||||
json!(expected_cwd.to_string())
|
||||
);
|
||||
assert_eq!(item.network, None);
|
||||
assert_eq!(item.file_system_sandbox_policy, None);
|
||||
assert_eq!(item.comp_hash, None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_item_rejects_legacy_policy_hydration_for_foreign_cwd() -> Result<()> {
|
||||
let foreign_cwd = if cfg!(windows) {
|
||||
"file:///tmp"
|
||||
} else {
|
||||
"file://server/share"
|
||||
};
|
||||
let item: TurnContextItem = serde_json::from_value(json!({
|
||||
"cwd": foreign_cwd,
|
||||
"approval_policy": "never",
|
||||
"sandbox_policy": { "type": "workspace-write" },
|
||||
"model": "gpt-5",
|
||||
"summary": "auto",
|
||||
}))?;
|
||||
|
||||
let err = item
|
||||
.permission_profile()
|
||||
.expect_err("foreign cwd should not hydrate a legacy permission profile");
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multi_agent_version_uses_newest_present_session_meta_value() -> Result<()> {
|
||||
let thread_id = ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?;
|
||||
@@ -5396,7 +5353,7 @@ mod tests {
|
||||
fn turn_context_item_serializes_network_when_present() -> Result<()> {
|
||||
let item = TurnContextItem {
|
||||
turn_id: None,
|
||||
cwd: PathUri::from_abs_path(&test_path_buf("/tmp").abs()),
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
|
||||
@@ -111,7 +111,7 @@ pub async fn extract_metadata_from_rollout(
|
||||
})?;
|
||||
let mut metadata = builder.build(default_provider);
|
||||
for item in &items {
|
||||
apply_rollout_item(&mut metadata, item, default_provider)?;
|
||||
apply_rollout_item(&mut metadata, item, default_provider);
|
||||
}
|
||||
if let Some(updated_at) = file_modified_time_utc(rollout_path).await {
|
||||
metadata.updated_at = updated_at;
|
||||
|
||||
@@ -69,8 +69,7 @@ async fn extract_metadata_from_rollout_uses_session_meta() {
|
||||
|
||||
let builder = builder_from_session_meta(&session_meta_line, path.as_path()).expect("builder");
|
||||
let mut expected = builder.build("openai");
|
||||
apply_rollout_item(&mut expected, &rollout_line.item, "openai")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut expected, &rollout_line.item, "openai");
|
||||
expected.updated_at = file_modified_time_utc(&path).await.expect("mtime");
|
||||
expected.recency_at = expected.updated_at;
|
||||
|
||||
|
||||
@@ -1827,7 +1827,6 @@ async fn resume_candidate_matches_cwd(
|
||||
| RolloutItem::Compacted(_)
|
||||
| RolloutItem::EventMsg(_) => None,
|
||||
})
|
||||
&& let Ok(latest_turn_context_cwd) = latest_turn_context_cwd.to_abs_path()
|
||||
{
|
||||
return cwd_matches(latest_turn_context_cwd.as_path(), cwd);
|
||||
}
|
||||
|
||||
@@ -16,10 +16,10 @@ pub fn apply_rollout_item(
|
||||
metadata: &mut ThreadMetadata,
|
||||
item: &RolloutItem,
|
||||
default_provider: &str,
|
||||
) -> std::io::Result<()> {
|
||||
) {
|
||||
match item {
|
||||
RolloutItem::SessionMeta(meta_line) => apply_session_meta_from_item(metadata, meta_line),
|
||||
RolloutItem::TurnContext(turn_ctx) => apply_turn_context(metadata, turn_ctx)?,
|
||||
RolloutItem::TurnContext(turn_ctx) => apply_turn_context(metadata, turn_ctx),
|
||||
RolloutItem::EventMsg(event) => apply_event_msg(metadata, event),
|
||||
RolloutItem::ResponseItem(item) => apply_response_item(metadata, item),
|
||||
RolloutItem::InterAgentCommunication(_) => {}
|
||||
@@ -28,7 +28,6 @@ pub fn apply_rollout_item(
|
||||
if metadata.model_provider.is_empty() {
|
||||
metadata.model_provider = default_provider.to_string();
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Return whether this rollout item can mutate thread metadata stored in SQLite.
|
||||
@@ -73,21 +72,15 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_turn_context(
|
||||
metadata: &mut ThreadMetadata,
|
||||
turn_ctx: &TurnContextItem,
|
||||
) -> std::io::Result<()> {
|
||||
if metadata.cwd.as_os_str().is_empty()
|
||||
&& let Ok(cwd) = turn_ctx.cwd.to_abs_path()
|
||||
{
|
||||
metadata.cwd = cwd.into_path_buf();
|
||||
fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) {
|
||||
if metadata.cwd.as_os_str().is_empty() {
|
||||
metadata.cwd = turn_ctx.cwd.clone().into_path_buf();
|
||||
}
|
||||
metadata.model = Some(turn_ctx.model.clone());
|
||||
metadata.reasoning_effort = turn_ctx.effort.clone();
|
||||
metadata.sandbox_policy =
|
||||
serde_json::to_string(&turn_ctx.permission_profile()?).unwrap_or_default();
|
||||
serde_json::to_string(&turn_ctx.permission_profile()).unwrap_or_default();
|
||||
metadata.approval_mode = enum_to_string(&turn_ctx.approval_policy);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) {
|
||||
@@ -201,8 +194,7 @@ mod tests {
|
||||
metadata: None,
|
||||
});
|
||||
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
assert_eq!(metadata.preview, None);
|
||||
@@ -221,8 +213,7 @@ mod tests {
|
||||
..Default::default()
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(
|
||||
metadata.first_user_message.as_deref(),
|
||||
@@ -244,8 +235,7 @@ mod tests {
|
||||
..Default::default()
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(
|
||||
metadata.first_user_message.as_deref(),
|
||||
@@ -270,8 +260,7 @@ mod tests {
|
||||
..Default::default()
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
assert_eq!(metadata.preview, None);
|
||||
@@ -297,8 +286,7 @@ mod tests {
|
||||
},
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &goal_item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &goal_item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark"));
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
@@ -313,8 +301,7 @@ mod tests {
|
||||
..Default::default()
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &user_item, "test-provider")
|
||||
.expect("rollout item should apply");
|
||||
apply_rollout_item(&mut metadata, &user_item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark"));
|
||||
assert_eq!(
|
||||
@@ -357,8 +344,7 @@ mod tests {
|
||||
git: None,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
apply_rollout_item(
|
||||
&mut metadata,
|
||||
&RolloutItem::TurnContext(TurnContextItem {
|
||||
@@ -387,8 +373,7 @@ mod tests {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
|
||||
assert_eq!(metadata.cwd, PathBuf::from("/child/worktree"));
|
||||
let permission_profile: PermissionProfile = PermissionProfile::Disabled;
|
||||
@@ -432,8 +417,7 @@ mod tests {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
metadata.sandbox_policy,
|
||||
@@ -473,8 +457,7 @@ mod tests {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
|
||||
assert_eq!(metadata.cwd, fallback_cwd);
|
||||
}
|
||||
@@ -511,8 +494,7 @@ mod tests {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
|
||||
assert_eq!(metadata.model.as_deref(), Some("gpt-5"));
|
||||
assert_eq!(metadata.reasoning_effort, Some(ReasoningEffort::High));
|
||||
@@ -548,8 +530,7 @@ mod tests {
|
||||
git: None,
|
||||
}),
|
||||
"test-provider",
|
||||
)
|
||||
.expect("rollout item should apply");
|
||||
);
|
||||
|
||||
assert_eq!(metadata.model, None);
|
||||
assert_eq!(metadata.reasoning_effort, None);
|
||||
|
||||
@@ -883,7 +883,7 @@ ON CONFLICT(id) DO UPDATE SET
|
||||
.unwrap_or_else(|| builder.build(&self.default_provider));
|
||||
metadata.rollout_path = builder.rollout_path.clone();
|
||||
for item in items {
|
||||
apply_rollout_item(&mut metadata, item, &self.default_provider)?;
|
||||
apply_rollout_item(&mut metadata, item, &self.default_provider);
|
||||
}
|
||||
if let Some(existing_metadata) = existing_metadata.as_ref() {
|
||||
metadata.prefer_existing_git_info(existing_metadata);
|
||||
|
||||
@@ -125,7 +125,7 @@ impl LiveThread {
|
||||
}
|
||||
}
|
||||
}
|
||||
let metadata_sync = ThreadMetadataSync::for_resume(¶ms)?;
|
||||
let metadata_sync = ThreadMetadataSync::for_resume(¶ms);
|
||||
Ok(Self {
|
||||
thread_id,
|
||||
thread_store,
|
||||
@@ -151,7 +151,7 @@ impl LiveThread {
|
||||
.metadata_sync
|
||||
.lock()
|
||||
.await
|
||||
.observe_appended_items(canonical_items.as_slice())?;
|
||||
.observe_appended_items(canonical_items.as_slice());
|
||||
if let Some(update) = update {
|
||||
self.thread_store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
|
||||
@@ -18,8 +18,6 @@ use crate::CreateThreadParams;
|
||||
use crate::GitInfoPatch;
|
||||
use crate::ResumeThreadParams;
|
||||
use crate::ThreadMetadataPatch;
|
||||
use crate::ThreadStoreError;
|
||||
use crate::ThreadStoreResult;
|
||||
|
||||
const IMAGE_ONLY_USER_MESSAGE_PLACEHOLDER: &str = "[Image]";
|
||||
#[cfg(not(test))]
|
||||
@@ -92,7 +90,7 @@ impl ThreadMetadataSync {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn for_resume(params: &ResumeThreadParams) -> ThreadStoreResult<Self> {
|
||||
pub(crate) fn for_resume(params: &ResumeThreadParams) -> Self {
|
||||
let mut sync = Self {
|
||||
thread_id: params.thread_id,
|
||||
cwd_seen: params
|
||||
@@ -110,11 +108,11 @@ impl ThreadMetadataSync {
|
||||
defer_resume_update_until_append: false,
|
||||
};
|
||||
if let Some(history) = params.history.as_deref() {
|
||||
let update = sync.observe_resume_history(history)?;
|
||||
let update = sync.observe_resume_history(history);
|
||||
sync.merge_pending_update(update);
|
||||
sync.defer_resume_update_until_append = sync.pending_update.is_some();
|
||||
}
|
||||
Ok(sync)
|
||||
sync
|
||||
}
|
||||
|
||||
pub(crate) fn take_pending_update(&self) -> Option<PendingThreadMetadataPatch> {
|
||||
@@ -150,7 +148,7 @@ impl ThreadMetadataSync {
|
||||
pub(crate) fn observe_appended_items(
|
||||
&mut self,
|
||||
items: &[RolloutItem],
|
||||
) -> ThreadStoreResult<Option<PendingThreadMetadataPatch>> {
|
||||
) -> Option<PendingThreadMetadataPatch> {
|
||||
self.defer_create_update_until_history_exists = false;
|
||||
self.defer_resume_update_until_append = false;
|
||||
let affects_metadata = items
|
||||
@@ -162,10 +160,7 @@ impl ThreadMetadataSync {
|
||||
let mut update = if affects_metadata {
|
||||
self.observe_items(items)?
|
||||
} else {
|
||||
Some(thread_updated_at_touch())
|
||||
};
|
||||
let Some(update) = update else {
|
||||
return Ok(None);
|
||||
thread_updated_at_touch()
|
||||
};
|
||||
if advances_recency {
|
||||
update.advance_recency_at = Some(Utc::now());
|
||||
@@ -180,15 +175,12 @@ impl ThreadMetadataSync {
|
||||
Instant::now().duration_since(last_touch) < THREAD_UPDATED_AT_TOUCH_INTERVAL
|
||||
})
|
||||
{
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
Ok(self.take_pending_update())
|
||||
self.take_pending_update()
|
||||
}
|
||||
|
||||
fn observe_items(
|
||||
&mut self,
|
||||
items: &[RolloutItem],
|
||||
) -> ThreadStoreResult<Option<ThreadMetadataPatch>> {
|
||||
fn observe_items(&mut self, items: &[RolloutItem]) -> Option<ThreadMetadataPatch> {
|
||||
self.observe_items_with_update(
|
||||
items,
|
||||
ThreadMetadataPatch {
|
||||
@@ -198,10 +190,7 @@ impl ThreadMetadataSync {
|
||||
)
|
||||
}
|
||||
|
||||
fn observe_resume_history(
|
||||
&mut self,
|
||||
items: &[RolloutItem],
|
||||
) -> ThreadStoreResult<Option<ThreadMetadataPatch>> {
|
||||
fn observe_resume_history(&mut self, items: &[RolloutItem]) -> Option<ThreadMetadataPatch> {
|
||||
self.observe_items_with_update(items, ThreadMetadataPatch::default())
|
||||
}
|
||||
|
||||
@@ -209,9 +198,9 @@ impl ThreadMetadataSync {
|
||||
&mut self,
|
||||
items: &[RolloutItem],
|
||||
mut update: ThreadMetadataPatch,
|
||||
) -> ThreadStoreResult<Option<ThreadMetadataPatch>> {
|
||||
) -> Option<ThreadMetadataPatch> {
|
||||
if items.is_empty() {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
for item in items {
|
||||
match item {
|
||||
@@ -244,23 +233,14 @@ impl ThreadMetadataSync {
|
||||
}
|
||||
}
|
||||
RolloutItem::TurnContext(turn_ctx) => {
|
||||
if !self.cwd_seen
|
||||
&& let Ok(cwd) = turn_ctx.cwd.to_abs_path()
|
||||
{
|
||||
if !self.cwd_seen {
|
||||
self.cwd_seen = true;
|
||||
update.cwd = Some(cwd.into_path_buf());
|
||||
update.cwd = Some(turn_ctx.cwd.clone().into_path_buf());
|
||||
}
|
||||
update.model = Some(turn_ctx.model.clone());
|
||||
update.reasoning_effort = turn_ctx.effort.clone();
|
||||
update.approval_mode = Some(turn_ctx.approval_policy);
|
||||
update.permission_profile =
|
||||
Some(turn_ctx.permission_profile().map_err(|err| {
|
||||
ThreadStoreError::Internal {
|
||||
message: format!(
|
||||
"failed to hydrate turn permission profile: {err}"
|
||||
),
|
||||
}
|
||||
})?);
|
||||
update.permission_profile = Some(turn_ctx.permission_profile());
|
||||
}
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user)) => {
|
||||
if let Some(preview) = user_message_preview(user) {
|
||||
@@ -302,7 +282,7 @@ impl ThreadMetadataSync {
|
||||
| RolloutItem::Compacted(_) => {}
|
||||
}
|
||||
}
|
||||
Ok(Some(update))
|
||||
Some(update)
|
||||
}
|
||||
|
||||
fn merge_pending_update(&mut self, update: Option<ThreadMetadataPatch>) {
|
||||
@@ -422,8 +402,7 @@ mod tests {
|
||||
RolloutItem::SessionMeta(session_meta(thread_id)),
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user_message("hello metadata"))),
|
||||
],
|
||||
))
|
||||
.expect("resume metadata should hydrate");
|
||||
));
|
||||
|
||||
let update = sync.take_pending_update().expect("pending metadata update");
|
||||
assert_eq!(
|
||||
@@ -462,8 +441,7 @@ mod tests {
|
||||
))),
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user_message("first user text"))),
|
||||
],
|
||||
))
|
||||
.expect("resume metadata should hydrate");
|
||||
));
|
||||
|
||||
let update = sync.take_pending_update().expect("pending metadata update");
|
||||
assert_eq!(update.patch.preview.as_deref(), Some("ship the refactor"));
|
||||
@@ -482,8 +460,7 @@ mod tests {
|
||||
vec![RolloutItem::EventMsg(EventMsg::UserMessage(user_message(
|
||||
"first user text",
|
||||
)))],
|
||||
))
|
||||
.expect("resume metadata should hydrate");
|
||||
));
|
||||
let pending = sync.take_pending_update().expect("pending resume metadata");
|
||||
sync.mark_pending_update_applied(&pending);
|
||||
|
||||
@@ -491,7 +468,6 @@ mod tests {
|
||||
.observe_appended_items(&[RolloutItem::EventMsg(EventMsg::UserMessage(user_message(
|
||||
"later user text",
|
||||
)))])
|
||||
.expect("appended metadata should hydrate")
|
||||
.expect("updated_at touch");
|
||||
|
||||
assert_eq!(update.patch.preview, None);
|
||||
@@ -503,8 +479,7 @@ mod tests {
|
||||
#[test]
|
||||
fn metadata_irrelevant_items_coalesce_updated_at_touches() {
|
||||
let thread_id = ThreadId::new();
|
||||
let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new()))
|
||||
.expect("resume metadata should hydrate");
|
||||
let mut sync = ThreadMetadataSync::for_resume(&resume_params(thread_id, Vec::new()));
|
||||
let item = RolloutItem::Compacted(CompactedItem {
|
||||
message: "compacted".to_string(),
|
||||
replacement_history: None,
|
||||
@@ -513,14 +488,12 @@ mod tests {
|
||||
|
||||
let first = sync
|
||||
.observe_appended_items(std::slice::from_ref(&item))
|
||||
.expect("appended metadata should hydrate")
|
||||
.expect("first touch should apply immediately");
|
||||
assert!(first.patch.updated_at.is_some());
|
||||
sync.mark_pending_update_applied(&first);
|
||||
|
||||
assert!(
|
||||
sync.observe_appended_items(std::slice::from_ref(&item))
|
||||
.expect("appended metadata should hydrate")
|
||||
.is_none(),
|
||||
"second touch inside the coalescing window should wait for a barrier"
|
||||
);
|
||||
@@ -560,8 +533,7 @@ mod tests {
|
||||
RolloutItem::SessionMeta(session_meta(thread_id)),
|
||||
RolloutItem::EventMsg(EventMsg::UserMessage(user_message("hello metadata"))),
|
||||
],
|
||||
))
|
||||
.expect("resume metadata should hydrate");
|
||||
));
|
||||
|
||||
assert!(
|
||||
sync.take_pending_update_for_existing_history().is_none(),
|
||||
@@ -571,7 +543,6 @@ mod tests {
|
||||
sync.observe_appended_items(&[RolloutItem::EventMsg(EventMsg::UserMessage(
|
||||
user_message("new append"),
|
||||
))])
|
||||
.expect("appended metadata should hydrate")
|
||||
.is_some(),
|
||||
"the first append should flush resume metadata together with append metadata"
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user