openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-03 19:53:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

address codex review feedback

Browse Source
This commit is contained in:
anais
2026-06-15 13:36:44 -07:00
parent 21a204fda8
commit 0a1bb6927d
5 changed files with 77 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/thread_processor.rs
-12
View File
@@ -3177,18 +3177,6 @@ impl ThreadRequestProcessor {
InitialHistory::Resumed(resumed) => {
let fallback_provider = config_snapshot.model_provider_id.as_str();
if let Some(mut stored_thread) = resume_source_thread {
if let Some(created_at) = resumed.history.iter().find_map(|item| match item {
RolloutItem::SessionMeta(session_meta) => {
chrono::DateTime::parse_from_rfc3339(&session_meta.meta.timestamp)
.ok()
.map(|timestamp| timestamp.with_timezone(&chrono::Utc))
}
_ => None,
}) {
// Resume does not advance updated_at until the next turn starts.
stored_thread.created_at = created_at;
stored_thread.updated_at = created_at;
}
if stored_thread.preview.is_empty() {
stored_thread.preview = preview_from_rollout_items(&resumed.history);
}
codex-rs/rollout/src/list.rs
+3 -4
View File
@@ -1778,7 +1778,6 @@ async fn find_thread_path_by_id_str_in_subdir(
_ => None,
};
let thread_id = ThreadId::from_string(id_str).ok();
let mut unverified_db_path = None;
let mut state_metadata_path = None;
let mut fallback_reason = state_db_ctx.is_none().then_some("db_unavailable");
if let Some(state_db_ctx) = state_db_ctx
@@ -1830,7 +1829,7 @@ async fn find_thread_path_by_id_str_in_subdir(
"state db returned rollout path for thread {id_str} that could not be verified: {}: {err}",
existing_db_path.display()
);
unverified_db_path = Some(existing_db_path);
fallback_reason = Some("unverified_path");
}
}
} else {
@@ -1900,7 +1899,7 @@ async fn find_thread_path_by_id_str_in_subdir(
}
}
Ok(found.or(unverified_db_path))
Ok(found)
}
async fn find_thread_path_by_id_str_in_any_subdir(
@@ -1954,7 +1953,7 @@ async fn find_thread_path_by_id_str_in_any_subdir(
"state db returned rollout path for thread {id_str} that could not be verified: {}: {err}",
existing_db_path.display()
);
return Ok(Some(existing_db_path));
fallback_reason = Some("unverified_path");
}
}
} else {
codex-rs/thread-store/src/local/list_threads.rs
+11 -5
View File
@@ -75,11 +75,6 @@ pub(super) async fn list_threads(
})
.collect::<Vec<_>>();
// Startup --last lookups use this internal mode only to resolve id/path.
if params.use_state_db_only && params.page_size == 1 && params.search_term.is_none() {
return Ok(ThreadPage { items, next_cursor });
}
let thread_ids = items
.iter()
.map(|thread| thread.thread_id)
@@ -95,6 +90,17 @@ pub(super) async fn list_threads(
}
}
}
for thread in &mut items {
if let Some(title) = names.get(&thread.thread_id).cloned() {
set_thread_name_from_title(thread, title);
}
}
// Startup --last lookups use this internal mode only to resolve id/path.
if params.use_state_db_only && params.page_size == 1 && params.search_term.is_none() {
return Ok(ThreadPage { items, next_cursor });
}
if names.len() < thread_ids.len()
&& let Ok(legacy_names) =
find_thread_names_by_ids(store.config.codex_home.as_path(), &thread_ids).await
codex-rs/thread-store/src/local/read_thread.rs
+32 -33
View File
@@ -38,35 +38,33 @@ pub(super) async fn read_thread(
store.config.codex_home.as_path(),
metadata.rollout_path.as_path(),
)))
&& (!params.include_history
|| sqlite_rollout_path_can_load_history_for_thread(
store,
&metadata.rollout_path,
thread_id,
)
.await)
{
let metadata_sandbox_policy = metadata.sandbox_policy.clone();
let rollout_thread = matching_rollout_thread_from_sqlite_path(
store,
metadata.rollout_path.as_path(),
thread_id,
params.include_archived,
)
.await;
let mut thread = stored_thread_from_sqlite_metadata(store, metadata).await;
if !params.include_history
&& let Some(rollout_path) = thread.rollout_path.clone()
&& let Ok(mut rollout_thread) = read_thread_from_rollout_path(store, rollout_path).await
&& rollout_thread.thread_id == thread_id
&& (params.include_archived || rollout_thread.archived_at.is_none())
&& !rollout_thread.preview.is_empty()
{
if thread.name.is_some() {
rollout_thread.name = thread.name;
if !params.include_history || rollout_thread.is_some() {
if let Some(mut rollout_thread) = rollout_thread
&& !rollout_thread.preview.is_empty()
{
if thread.name.is_some() {
rollout_thread.name = thread.name;
}
rollout_thread.git_info = thread.git_info;
rollout_thread.permission_profile = permission_profile_from_metadata_value(
&metadata_sandbox_policy,
rollout_thread.cwd.as_path(),
);
thread = rollout_thread;
}
rollout_thread.git_info = thread.git_info;
rollout_thread.permission_profile = permission_profile_from_metadata_value(
&metadata_sandbox_policy,
rollout_thread.cwd.as_path(),
);
thread = rollout_thread;
attach_history_if_requested(&mut thread, params.include_history).await?;
return Ok(thread);
}
attach_history_if_requested(&mut thread, params.include_history).await?;
return Ok(thread);
}
let path = resolve_rollout_path(store, thread_id, params.include_archived)
@@ -85,20 +83,21 @@ pub(super) async fn read_thread(
Ok(thread)
}
async fn sqlite_rollout_path_can_load_history_for_thread(
async fn matching_rollout_thread_from_sqlite_path(
store: &LocalThreadStore,
path: &std::path::Path,
thread_id: codex_protocol::ThreadId,
) -> bool {
if codex_rollout::existing_rollout_path(path).await.is_none() {
return false;
}
// SQLite metadata can outlive a moved/recreated rollout path. When history is
// requested, verify the path still resolves to the requested thread before
// trusting it as the source replay.
include_archived: bool,
) -> Option<StoredThread> {
codex_rollout::existing_rollout_path(path).await.as_ref()?;
// SQLite metadata can outlive a moved or recreated rollout path. Verify the
// path still resolves to the requested thread before trusting its summary or history.
read_thread_from_rollout_path(store, path.to_path_buf())
.await
.is_ok_and(|thread| thread.thread_id == thread_id)
.ok()
.filter(|thread| {
thread.thread_id == thread_id && (include_archived || thread.archived_at.is_none())
})
}
pub(super) async fn read_thread_by_rollout_path(
codex-rs/tui/src/session_resume.rs
+31 -24
View File
@@ -98,16 +98,18 @@ pub(crate) async fn resolve_cwd_for_resume_or_fork(
action: CwdPromptAction,
allow_prompt: bool,
) -> color_eyre::Result<ResolveCwdOutcome> {
let history_cwd = match target_session.cwd.as_deref() {
Some(cwd) => Some(cwd.to_path_buf()),
None => {
read_session_cwd(
state_db_ctx,
target_session.thread_id,
target_session.path.as_deref(),
)
.await
}
let history_cwd = if target_session.path.is_some() {
read_session_cwd(
state_db_ctx,
target_session.thread_id,
target_session.path.as_deref(),
)
.await
.or_else(|| target_session.cwd.clone())
} else if let Some(cwd) = target_session.cwd.as_ref() {
Some(cwd.clone())
} else {
read_session_cwd(state_db_ctx, target_session.thread_id, /*path*/ None).await
};
let Some(history_cwd) = history_cwd else {
return Ok(ResolveCwdOutcome::Continue(None));
@@ -133,25 +135,30 @@ async fn read_session_cwd(
thread_id: ThreadId,
path: Option<&Path>,
) -> Option<PathBuf> {
if let Some(path) = path {
match read_rollout_resume_state(path).await {
Ok(state) => {
if state.cwd.is_some() {
return state.cwd;
}
}
Err(err) => {
let rollout_path = path.display().to_string();
tracing::warn!(
%rollout_path,
%err,
"Failed to read session metadata from rollout"
);
}
}
}
if let Some(state_db_ctx) = state_db_ctx
&& let Ok(Some(metadata)) = state_db_ctx.get_thread(thread_id).await
{
return Some(metadata.cwd);
}
let path = path?;
match read_rollout_resume_state(path).await {
Ok(state) => state.cwd,
Err(err) => {
let rollout_path = path.display().to_string();
tracing::warn!(
%rollout_path,
%err,
"Failed to read session metadata from rollout"
);
None
}
}
None
}
pub(crate) fn cwds_differ(current_cwd: &Path, session_cwd: &Path) -> bool {