mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
## Why A thread ID alone does not identify the backend that owns a session. Resume and fork selections must retain that ownership across connection replacement so they cannot operate on another backend, even when thread IDs match. ## What changed - Add a process-local `BackendId` to app-server sessions and session targets, preserving it across reconnects and auxiliary picker connections. - Reject selections from a different backend before changing session state or sending resume, fork, or unarchive requests. - Disable local-daemon fallback to an embedded server for auxiliary pickers and after selecting a session, while retaining fallback for fresh startup. ## Testing Add regression coverage for foreign selections with matching thread IDs, startup resume and fork rejection before requests, shared ownership across picker connections, and fallback eligibility. GitOrigin-RevId: 05a3ca0a901a8dc24337d130548aa2c048b4d586
223 lines
7.7 KiB
Rust
223 lines
7.7 KiB
Rust
use super::*;
|
|
use crate::legacy_core::config::ConfigBuilder;
|
|
use app_test_support::create_fake_rollout;
|
|
use app_test_support::rollout_path;
|
|
use codex_config::LoaderOverrides;
|
|
use codex_protocol::ThreadId;
|
|
use pretty_assertions::assert_eq;
|
|
use std::cell::Cell;
|
|
use tempfile::TempDir;
|
|
|
|
async fn start_session(
|
|
app_server: &mut AppServerSession,
|
|
config: &Config,
|
|
target: &SessionTarget,
|
|
action: SessionStartAction,
|
|
confirm: impl AsyncFnOnce() -> Result<UnarchiveChoice>,
|
|
) -> Result<SessionStartOutcome> {
|
|
let local_settings = LocalSettings::from(config);
|
|
let initial_result = action
|
|
.start(app_server, config, &local_settings, target)
|
|
.await;
|
|
complete_session_start(
|
|
app_server,
|
|
SessionStartConfig {
|
|
config,
|
|
local_settings: &local_settings,
|
|
},
|
|
&crate::AppServerTarget::Embedded,
|
|
target,
|
|
action,
|
|
initial_result,
|
|
confirm,
|
|
)
|
|
.await
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn archived_session_requires_confirmation_before_resume_or_fork() -> Result<()> {
|
|
for action in [
|
|
SessionStartAction::Resume(
|
|
ResumeModelSettings::RestoreFromThread,
|
|
crate::resume_permissions::ResumePermissions::default(),
|
|
),
|
|
SessionStartAction::Fork(crate::app_server_session::ForkPermissionMode::InheritSaved),
|
|
] {
|
|
let codex_home = TempDir::new()?;
|
|
let config = ConfigBuilder::default()
|
|
.codex_home(codex_home.path().to_path_buf())
|
|
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
|
.build()
|
|
.await?;
|
|
let timestamp = "2025-01-05T12-00-00";
|
|
let id = create_fake_rollout(
|
|
codex_home.path(),
|
|
timestamp,
|
|
"2025-01-05T12:00:00Z",
|
|
"Archived saved user message",
|
|
Some(config.model_provider_id.as_str()),
|
|
/*git_info*/ None,
|
|
)
|
|
.expect("create source rollout");
|
|
let active_path = rollout_path(codex_home.path(), timestamp, &id);
|
|
let archived_dir = codex_home.path().join("archived_sessions");
|
|
std::fs::create_dir_all(&archived_dir)?;
|
|
let archived_path = archived_dir.join(active_path.file_name().unwrap());
|
|
std::fs::rename(&active_path, &archived_path)?;
|
|
let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?;
|
|
let backend_id = app_server.backend_id();
|
|
let target = SessionTarget {
|
|
backend_id,
|
|
path: Some(archived_path.clone()),
|
|
thread_id: ThreadId::from_string(&id)?,
|
|
cwd: None,
|
|
history_mode: None,
|
|
};
|
|
let prompts = Cell::new(0);
|
|
let local_settings = LocalSettings::from(&config);
|
|
|
|
let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500")?;
|
|
for server_target in [
|
|
crate::AppServerTarget::LocalDaemon {
|
|
allow_embedded_fallback: true,
|
|
endpoint: endpoint.clone(),
|
|
},
|
|
crate::AppServerTarget::Remote { endpoint },
|
|
crate::AppServerTarget::Embedded,
|
|
] {
|
|
for choice in [UnarchiveChoice::Cancel, UnarchiveChoice::Quit] {
|
|
let initial = action
|
|
.start(&mut app_server, &config, &local_settings, &target)
|
|
.await;
|
|
let outcome = complete_session_start(
|
|
&mut app_server,
|
|
SessionStartConfig {
|
|
config: &config,
|
|
local_settings: &local_settings,
|
|
},
|
|
&server_target,
|
|
&target,
|
|
action,
|
|
initial,
|
|
async || {
|
|
prompts.set(prompts.get() + 1);
|
|
Ok(choice)
|
|
},
|
|
)
|
|
.await?;
|
|
assert_eq!(
|
|
matches!(outcome, SessionStartOutcome::CommandCenter),
|
|
choice == UnarchiveChoice::Cancel
|
|
&& !matches!(server_target, crate::AppServerTarget::Embedded),
|
|
);
|
|
assert_eq!(
|
|
(archived_path.exists(), active_path.exists()),
|
|
(true, false)
|
|
);
|
|
assert!(
|
|
app_server
|
|
.thread_loaded_list(Default::default())
|
|
.await?
|
|
.data
|
|
.is_empty()
|
|
);
|
|
}
|
|
}
|
|
|
|
assert_eq!(prompts.get(), 6);
|
|
let SessionStartOutcome::Started(started) =
|
|
start_session(&mut app_server, &config, &target, action, async || {
|
|
prompts.set(prompts.get() + 1);
|
|
Ok(UnarchiveChoice::Unarchive)
|
|
})
|
|
.await?
|
|
else {
|
|
panic!("confirmed session should start")
|
|
};
|
|
assert_eq!(
|
|
(prompts.get(), archived_path.exists(), active_path.exists()),
|
|
(7, false, true)
|
|
);
|
|
match action {
|
|
SessionStartAction::Resume(..) => {
|
|
assert_eq!(started.session.thread_id, target.thread_id)
|
|
}
|
|
SessionStartAction::Fork(_) => {
|
|
assert_ne!(started.session.thread_id, target.thread_id);
|
|
assert_eq!(
|
|
app_server
|
|
.thread_read(started.session.thread_id, /*include_turns*/ false)
|
|
.await?
|
|
.forked_from_id,
|
|
Some(id)
|
|
);
|
|
}
|
|
}
|
|
|
|
let SessionStartOutcome::Started(resumed) = start_session(
|
|
&mut app_server,
|
|
&config,
|
|
&target,
|
|
SessionStartAction::Resume(
|
|
ResumeModelSettings::RestoreFromThread,
|
|
crate::resume_permissions::ResumePermissions::default(),
|
|
),
|
|
async || panic!("active sessions must not prompt"),
|
|
)
|
|
.await?
|
|
else {
|
|
panic!("active session should start")
|
|
};
|
|
assert_eq!(resumed.session.thread_id, target.thread_id);
|
|
|
|
let missing = SessionTarget {
|
|
backend_id,
|
|
path: None,
|
|
thread_id: ThreadId::new(),
|
|
cwd: None,
|
|
history_mode: None,
|
|
};
|
|
let error = start_session(&mut app_server, &config, &missing, action, async || {
|
|
panic!("unrelated failures must not prompt")
|
|
})
|
|
.await
|
|
.expect_err("missing session should fail");
|
|
assert!(
|
|
error
|
|
.to_string()
|
|
.starts_with(&format!("Failed to {} session", action.verb()))
|
|
);
|
|
app_server.shutdown().await?;
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
#[test]
|
|
fn session_start_error_surfaces_archived_guidance_without_rollout_path() {
|
|
let thread_id =
|
|
ThreadId::from_string("019e72f4-e09a-70f2-b2c2-a153a57b8cc0").expect("thread id");
|
|
let target_session = SessionTarget {
|
|
backend_id: crate::backend_id::BackendId::new(),
|
|
path: Some(std::path::PathBuf::from(
|
|
"/Users/me/.codex/archived_sessions/rollout.jsonl",
|
|
)),
|
|
thread_id,
|
|
cwd: None,
|
|
history_mode: None,
|
|
};
|
|
let expected = format!(
|
|
"session {thread_id} is archived. Run `codex unarchive {thread_id}` to unarchive it first."
|
|
);
|
|
|
|
for action in ["resume", "fork"] {
|
|
let err = color_eyre::eyre::eyre!(
|
|
"thread/{action} failed during TUI bootstrap: thread/{action} failed: {expected} (code -32600)"
|
|
);
|
|
|
|
assert_eq!(
|
|
session_start_error(action, &target_session, err).to_string(),
|
|
expected
|
|
);
|
|
}
|
|
}
|