mirror of
https://github.com/openai/codex.git
synced 2026-09-10 20:26:47 +00:00
Fix alarm-only threads in resume picker
This commit is contained in:
20
codex-rs/rollout/src/alarm_sidecar.rs
Normal file
20
codex-rs/rollout/src/alarm_sidecar.rs
Normal file
@@ -0,0 +1,20 @@
|
||||
//! Helpers for locating alarm sidecars and surfacing scheduler-only threads in
|
||||
//! session listings.
|
||||
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
const ALARM_THREAD_PREVIEW: &str = "(alarm configured)";
|
||||
|
||||
pub(crate) fn alarm_sidecar_path_for_rollout(rollout_path: &Path) -> PathBuf {
|
||||
PathBuf::from(format!("{}.alarms.json", rollout_path.display()))
|
||||
}
|
||||
|
||||
pub(crate) async fn thread_preview_from_alarm_sidecar(rollout_path: &Path) -> Option<String> {
|
||||
let sidecar_path = alarm_sidecar_path_for_rollout(rollout_path);
|
||||
tokio::fs::try_exists(sidecar_path)
|
||||
.await
|
||||
.ok()
|
||||
.filter(|exists| *exists)
|
||||
.map(|_| ALARM_THREAD_PREVIEW.to_string())
|
||||
}
|
||||
@@ -4,6 +4,7 @@ use std::sync::LazyLock;
|
||||
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
|
||||
mod alarm_sidecar;
|
||||
pub mod config;
|
||||
pub mod list;
|
||||
pub mod metadata;
|
||||
|
||||
@@ -17,6 +17,7 @@ use uuid::Uuid;
|
||||
|
||||
use super::ARCHIVED_SESSIONS_SUBDIR;
|
||||
use super::SESSIONS_SUBDIR;
|
||||
use super::alarm_sidecar::thread_preview_from_alarm_sidecar;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::state_db;
|
||||
use codex_file_search as file_search;
|
||||
@@ -713,8 +714,12 @@ async fn build_thread_item(
|
||||
{
|
||||
return None;
|
||||
}
|
||||
// Apply filters: must have session meta and at least one user message event
|
||||
if summary.saw_session_meta && summary.saw_user_event {
|
||||
let alarm_preview = if summary.saw_user_event {
|
||||
None
|
||||
} else {
|
||||
thread_preview_from_alarm_sidecar(&path).await
|
||||
};
|
||||
if summary.saw_session_meta && (summary.saw_user_event || alarm_preview.is_some()) {
|
||||
let HeadTailSummary {
|
||||
thread_id,
|
||||
first_user_message,
|
||||
@@ -737,7 +742,7 @@ async fn build_thread_item(
|
||||
return Some(ThreadItem {
|
||||
path,
|
||||
thread_id,
|
||||
first_user_message,
|
||||
first_user_message: first_user_message.or(alarm_preview),
|
||||
cwd,
|
||||
git_branch,
|
||||
git_sha,
|
||||
|
||||
@@ -31,6 +31,23 @@ fn test_config(codex_home: &Path) -> RolloutConfig {
|
||||
}
|
||||
|
||||
fn write_session_file(root: &Path, ts: &str, uuid: Uuid) -> std::io::Result<PathBuf> {
|
||||
write_session_file_with_user_message(root, ts, uuid, /*include_user_message*/ true)
|
||||
}
|
||||
|
||||
fn write_session_file_without_user_message(
|
||||
root: &Path,
|
||||
ts: &str,
|
||||
uuid: Uuid,
|
||||
) -> std::io::Result<PathBuf> {
|
||||
write_session_file_with_user_message(root, ts, uuid, /*include_user_message*/ false)
|
||||
}
|
||||
|
||||
fn write_session_file_with_user_message(
|
||||
root: &Path,
|
||||
ts: &str,
|
||||
uuid: Uuid,
|
||||
include_user_message: bool,
|
||||
) -> std::io::Result<PathBuf> {
|
||||
let day_dir = root.join("sessions/2025/01/03");
|
||||
fs::create_dir_all(&day_dir)?;
|
||||
let path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl"));
|
||||
@@ -49,16 +66,18 @@ fn write_session_file(root: &Path, ts: &str, uuid: Uuid) -> std::io::Result<Path
|
||||
},
|
||||
});
|
||||
writeln!(file, "{meta}")?;
|
||||
let user_event = serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "event_msg",
|
||||
"payload": {
|
||||
"type": "user_message",
|
||||
"message": "Hello from user",
|
||||
"kind": "plain",
|
||||
},
|
||||
});
|
||||
writeln!(file, "{user_event}")?;
|
||||
if include_user_message {
|
||||
let user_event = serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "event_msg",
|
||||
"payload": {
|
||||
"type": "user_message",
|
||||
"message": "Hello from user",
|
||||
"kind": "plain",
|
||||
},
|
||||
});
|
||||
writeln!(file, "{user_event}")?;
|
||||
}
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
@@ -437,6 +456,65 @@ async fn list_threads_db_enabled_repairs_stale_rollout_paths() -> std::io::Resul
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_threads_includes_alarm_only_sessions_without_user_messages() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
|
||||
let uuid = Uuid::from_u128(9014);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path = write_session_file_without_user_message(home.path(), "2025-01-03T14-00-00", uuid)?;
|
||||
fs::write(format!("{}.alarms.json", path.display()), "[]")?;
|
||||
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
home.path().to_path_buf(),
|
||||
config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
runtime
|
||||
.mark_backfill_complete(/*last_watermark*/ None)
|
||||
.await
|
||||
.expect("backfill should be complete");
|
||||
let created_at = chrono::Utc
|
||||
.with_ymd_and_hms(2025, 1, 3, 14, 0, 0)
|
||||
.single()
|
||||
.expect("valid datetime");
|
||||
let mut builder = codex_state::ThreadMetadataBuilder::new(
|
||||
thread_id,
|
||||
path.clone(),
|
||||
created_at,
|
||||
SessionSource::Cli,
|
||||
);
|
||||
builder.model_provider = Some(config.model_provider_id.clone());
|
||||
builder.cwd = home.path().to_path_buf();
|
||||
let metadata = builder.build(config.model_provider_id.as_str());
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("state db upsert should succeed");
|
||||
|
||||
let default_provider = config.model_provider_id.clone();
|
||||
let page = RolloutRecorder::list_threads(
|
||||
&config,
|
||||
/*page_size*/ 1,
|
||||
/*cursor*/ None,
|
||||
ThreadSortKey::CreatedAt,
|
||||
&[],
|
||||
/*model_providers*/ None,
|
||||
default_provider.as_str(),
|
||||
/*search_term*/ None,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(page.items.len(), 1);
|
||||
assert_eq!(page.items[0].path, path);
|
||||
assert_eq!(
|
||||
page.items[0].first_user_message.as_deref(),
|
||||
Some("(alarm configured)")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resume_candidate_matches_cwd_reads_latest_turn_context() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::alarm_sidecar::thread_preview_from_alarm_sidecar;
|
||||
use crate::config::RolloutConfig;
|
||||
use crate::config::RolloutConfigView;
|
||||
use crate::list::Cursor;
|
||||
@@ -243,11 +244,22 @@ pub async fn list_threads_db(
|
||||
{
|
||||
Ok(mut page) => {
|
||||
let mut valid_items = Vec::with_capacity(page.items.len());
|
||||
for item in page.items {
|
||||
for mut item in page.items {
|
||||
if tokio::fs::try_exists(&item.rollout_path)
|
||||
.await
|
||||
.unwrap_or(false)
|
||||
{
|
||||
let missing_preview =
|
||||
item.first_user_message.as_deref().is_none_or(str::is_empty);
|
||||
if missing_preview {
|
||||
if let Some(alarm_preview) =
|
||||
thread_preview_from_alarm_sidecar(&item.rollout_path).await
|
||||
{
|
||||
item.first_user_message = Some(alarm_preview);
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
valid_items.push(item);
|
||||
} else {
|
||||
warn!(
|
||||
|
||||
@@ -883,7 +883,6 @@ pub(super) fn push_thread_filters<'a>(
|
||||
} else {
|
||||
builder.push(" AND archived = 0");
|
||||
}
|
||||
builder.push(" AND first_user_message <> ''");
|
||||
if !allowed_sources.is_empty() {
|
||||
builder.push(" AND source IN (");
|
||||
let mut separated = builder.separated(", ");
|
||||
|
||||
@@ -252,14 +252,7 @@ async fn run_session_picker_with_loader(
|
||||
ProviderFilter::MatchDefault(config.model_provider_id.to_string())
|
||||
};
|
||||
let codex_home = config.codex_home.as_path();
|
||||
let filter_cwd = if show_all || is_remote {
|
||||
// Remote sessions live in the server's filesystem namespace, so the client
|
||||
// process cwd is not a meaningful row filter. If the user provided an
|
||||
// explicit remote --cd, filtering is handled server-side in thread/list.
|
||||
None
|
||||
} else {
|
||||
std::env::current_dir().ok()
|
||||
};
|
||||
let filter_cwd = picker_filter_cwd(config, show_all, is_remote);
|
||||
|
||||
let mut state = PickerState::new(
|
||||
codex_home.to_path_buf(),
|
||||
@@ -1147,6 +1140,17 @@ fn thread_list_params(
|
||||
}
|
||||
}
|
||||
|
||||
fn picker_filter_cwd(config: &Config, show_all: bool, is_remote: bool) -> Option<PathBuf> {
|
||||
if show_all || is_remote {
|
||||
// Remote sessions live in the server's filesystem namespace, so the client
|
||||
// process cwd is not a meaningful row filter. If the user provided an
|
||||
// explicit remote --cd, filtering is handled server-side in thread/list.
|
||||
None
|
||||
} else {
|
||||
Some(config.cwd.to_path_buf())
|
||||
}
|
||||
}
|
||||
|
||||
fn paths_match(a: &Path, b: &Path) -> bool {
|
||||
if let (Ok(ca), Ok(cb)) = (
|
||||
path_utils::normalize_for_path_comparison(a),
|
||||
@@ -1645,6 +1649,7 @@ fn column_visibility(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use chrono::Duration;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_protocol::ThreadId;
|
||||
|
||||
use crossterm::event::KeyCode;
|
||||
@@ -1689,6 +1694,15 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
async fn build_config(temp_dir: &tempfile::TempDir, cwd: &Path) -> Config {
|
||||
ConfigBuilder::default()
|
||||
.codex_home(temp_dir.path().to_path_buf())
|
||||
.fallback_cwd(Some(cwd.to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("config should build")
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn set_rollout_mtime(path: &Path, updated_at: DateTime<Utc>) {
|
||||
let times = FileTimes::new().set_modified(updated_at.into());
|
||||
@@ -1896,6 +1910,32 @@ mod tests {
|
||||
assert_eq!(params.source_kinds, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_picker_filter_uses_config_cwd() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let config = build_config(&temp_dir, Path::new("/tmp/config-cwd")).await;
|
||||
|
||||
assert_eq!(
|
||||
picker_filter_cwd(&config, /*show_all*/ false, /*is_remote*/ false),
|
||||
Some(PathBuf::from("/tmp/config-cwd"))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn picker_filter_cwd_is_disabled_for_show_all_and_remote() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let config = build_config(&temp_dir, Path::new("/tmp/config-cwd")).await;
|
||||
|
||||
assert_eq!(
|
||||
picker_filter_cwd(&config, /*show_all*/ true, /*is_remote*/ false),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
picker_filter_cwd(&config, /*show_all*/ false, /*is_remote*/ true),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_picker_does_not_filter_rows_by_local_cwd() {
|
||||
let loader: PageLoader = Arc::new(|_| {});
|
||||
|
||||
Reference in New Issue
Block a user