mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Revert state DB injection and agent graph store (#21481)
## Why Reverts #20689 to restore the previous optional state DB plumbing. The conflict resolution keeps the newer installation ID and session/thread identity changes that landed after #20689, while removing the mandatory state DB and agent graph store dependency from ThreadManager construction. ## What changed - Restored `Option<StateDbHandle>` through app-server, MCP server, prompt debug, and test entry points. - Removed the `codex-core` dependency on `codex-agent-graph-store` and reverted descendant lookup back to the existing state DB path when available. - Kept newer `installation_id` forwarding by passing it beside the optional DB handle. - Kept local thread-name updates working when the optional state DB handle is absent. ## Validation - `git diff --check` - `cargo test -p codex-thread-store` - `cargo test -p codex-state -p codex-rollout -p codex-app-server-protocol` - Attempted `env CARGO_INCREMENTAL=0 cargo test -p codex-core -p codex-app-server -p codex-app-server-client -p codex-mcp-server -p codex-thread-manager-sample -p codex-tui`; blocked locally by a rustc ICE while compiling `v8 v146.4.0` with `rustc 1.93.0 (254b59607 2026-01-19)` on `aarch64-apple-darwin`.
This commit is contained in:
@@ -15,9 +15,7 @@ use anyhow::anyhow;
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::agent_graph_store_from_state_db;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::init_state_db_from_config;
|
||||
use codex_core::resolve_installation_id;
|
||||
use codex_core::shell::Shell;
|
||||
use codex_core::shell::get_shell_by_model_provided_path;
|
||||
@@ -426,33 +424,19 @@ impl TestCodexBuilder {
|
||||
environment_manager: Arc<codex_exec_server::EnvironmentManager>,
|
||||
) -> anyhow::Result<TestCodex> {
|
||||
let auth = self.auth.clone();
|
||||
let thread_manager = if config.model_catalog.is_some() {
|
||||
let state_db = init_state_db_from_config(&config)
|
||||
.await
|
||||
.expect("test codex requires state db");
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
let agent_graph_store = agent_graph_store_from_state_db(state_db.clone());
|
||||
let installation_id = resolve_installation_id(&config.codex_home).await?;
|
||||
ThreadManager::new(
|
||||
&config,
|
||||
codex_core::test_support::auth_manager_from_auth(auth.clone()),
|
||||
SessionSource::Exec,
|
||||
Arc::clone(&environment_manager),
|
||||
/*analytics_events_client*/ None,
|
||||
state_db,
|
||||
thread_store,
|
||||
agent_graph_store,
|
||||
installation_id,
|
||||
)
|
||||
} else {
|
||||
codex_core::test_support::thread_manager_with_models_provider_and_home(
|
||||
auth.clone(),
|
||||
config.model_provider.clone(),
|
||||
config.codex_home.to_path_buf(),
|
||||
Arc::clone(&environment_manager),
|
||||
)
|
||||
.await
|
||||
};
|
||||
let state_db = codex_core::init_state_db(&config).await;
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
let installation_id = resolve_installation_id(&config.codex_home).await?;
|
||||
let thread_manager = ThreadManager::new(
|
||||
&config,
|
||||
codex_core::test_support::auth_manager_from_auth(auth.clone()),
|
||||
SessionSource::Exec,
|
||||
Arc::clone(&environment_manager),
|
||||
/*analytics_events_client*/ None,
|
||||
thread_store,
|
||||
state_db.clone(),
|
||||
installation_id,
|
||||
);
|
||||
let thread_manager = Arc::new(thread_manager);
|
||||
let user_shell_override = self.user_shell_override.clone();
|
||||
|
||||
|
||||
@@ -5,8 +5,6 @@ use codex_core::NewThread;
|
||||
use codex_core::Prompt;
|
||||
use codex_core::ResponseEvent;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::agent_graph_store_from_state_db;
|
||||
use codex_core::init_state_db_from_config;
|
||||
use codex_core::resolve_installation_id;
|
||||
use codex_core::thread_store_from_config;
|
||||
use codex_features::Feature;
|
||||
@@ -1116,11 +1114,6 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() {
|
||||
Ok(None) => panic!("No CodexAuth found in codex_home"),
|
||||
Err(e) => panic!("Failed to load CodexAuth: {e}"),
|
||||
};
|
||||
let state_db = init_state_db_from_config(&config)
|
||||
.await
|
||||
.expect("client test requires state db");
|
||||
let thread_store = thread_store_from_config(&config, state_db.clone());
|
||||
let agent_graph_store = agent_graph_store_from_state_db(state_db.clone());
|
||||
let installation_id = resolve_installation_id(&config.codex_home)
|
||||
.await
|
||||
.expect("resolve installation id");
|
||||
@@ -1130,9 +1123,8 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() {
|
||||
SessionSource::Exec,
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
/*analytics_events_client*/ None,
|
||||
state_db,
|
||||
thread_store,
|
||||
agent_graph_store,
|
||||
thread_store_from_config(&config, /*state_db*/ None),
|
||||
/*state_db*/ None,
|
||||
installation_id,
|
||||
);
|
||||
let NewThread { thread: codex, .. } = thread_manager
|
||||
|
||||
@@ -13,8 +13,6 @@ use codex_protocol::protocol::SessionMeta;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use codex_rollout::RolloutConfig;
|
||||
use codex_rollout::state_db::StateDbHandle;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
@@ -28,27 +26,6 @@ async fn read_config_toml(codex_home: &Path) -> io::Result<ConfigToml> {
|
||||
toml::from_str(&contents).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
|
||||
}
|
||||
|
||||
async fn state_db_for_test(codex_home: &Path) -> io::Result<StateDbHandle> {
|
||||
let config = RolloutConfig {
|
||||
codex_home: codex_home.to_path_buf(),
|
||||
sqlite_home: codex_home.to_path_buf(),
|
||||
cwd: codex_home.to_path_buf(),
|
||||
model_provider_id: "openai".to_string(),
|
||||
generate_memories: false,
|
||||
};
|
||||
codex_rollout::state_db::try_init(&config)
|
||||
.await
|
||||
.map_err(io::Error::other)
|
||||
}
|
||||
|
||||
async fn run_migration(
|
||||
codex_home: &Path,
|
||||
config_toml: &ConfigToml,
|
||||
) -> io::Result<PersonalityMigrationStatus> {
|
||||
let state_db = state_db_for_test(codex_home).await?;
|
||||
maybe_migrate_personality(codex_home, config_toml, state_db).await
|
||||
}
|
||||
|
||||
async fn write_session_with_user_event(codex_home: &Path) -> io::Result<()> {
|
||||
let thread_id = ThreadId::new();
|
||||
let dir = codex_home
|
||||
@@ -166,7 +143,8 @@ async fn migration_marker_exists_no_sessions_no_change() -> io::Result<()> {
|
||||
let marker_path = temp.path().join(PERSONALITY_MIGRATION_FILENAME);
|
||||
tokio::fs::write(&marker_path, "v1\n").await?;
|
||||
|
||||
let status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedMarker);
|
||||
assert_eq!(
|
||||
@@ -180,7 +158,8 @@ async fn migration_marker_exists_no_sessions_no_change() -> io::Result<()> {
|
||||
async fn no_marker_no_sessions_no_change() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
|
||||
let status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedNoSessions);
|
||||
assert_eq!(
|
||||
@@ -199,7 +178,8 @@ async fn no_marker_sessions_sets_personality() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
|
||||
let status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::Applied);
|
||||
assert_eq!(
|
||||
@@ -219,7 +199,7 @@ async fn no_marker_sessions_preserves_existing_config_fields() -> io::Result<()>
|
||||
tokio::fs::write(temp.path().join("config.toml"), "model = \"gpt-5.4\"\n").await?;
|
||||
let config_toml = read_config_toml(temp.path()).await?;
|
||||
|
||||
let status = run_migration(temp.path(), &config_toml).await?;
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml, /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::Applied);
|
||||
let persisted = read_config_toml(temp.path()).await?;
|
||||
@@ -233,7 +213,8 @@ async fn no_marker_meta_only_rollout_is_treated_as_no_sessions() -> io::Result<(
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_meta_only(temp.path()).await?;
|
||||
|
||||
let status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedNoSessions);
|
||||
assert_eq!(
|
||||
@@ -253,7 +234,7 @@ async fn no_marker_explicit_global_personality_skips_migration() -> io::Result<(
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
let config_toml = parse_config_toml("personality = \"friendly\"\n")?;
|
||||
|
||||
let status = run_migration(temp.path(), &config_toml).await?;
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml, /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(
|
||||
status,
|
||||
@@ -283,7 +264,7 @@ personality = "friendly"
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let status = run_migration(temp.path(), &config_toml).await?;
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml, /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(
|
||||
status,
|
||||
@@ -306,7 +287,7 @@ async fn marker_short_circuits_invalid_profile_resolution() -> io::Result<()> {
|
||||
tokio::fs::write(temp.path().join(PERSONALITY_MIGRATION_FILENAME), "v1\n").await?;
|
||||
let config_toml = parse_config_toml("profile = \"missing\"\n")?;
|
||||
|
||||
let status = run_migration(temp.path(), &config_toml).await?;
|
||||
let status = maybe_migrate_personality(temp.path(), &config_toml, /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::SkippedMarker);
|
||||
Ok(())
|
||||
@@ -317,7 +298,7 @@ async fn invalid_selected_profile_returns_error_and_does_not_write_marker() -> i
|
||||
let temp = TempDir::new()?;
|
||||
let config_toml = parse_config_toml("profile = \"missing\"\n")?;
|
||||
|
||||
let err = run_migration(temp.path(), &config_toml)
|
||||
let err = maybe_migrate_personality(temp.path(), &config_toml, /*state_db*/ None)
|
||||
.await
|
||||
.expect_err("missing profile should fail");
|
||||
|
||||
@@ -334,8 +315,10 @@ async fn applied_migration_is_idempotent_on_second_run() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_session_with_user_event(temp.path()).await?;
|
||||
|
||||
let first_status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let second_status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let first_status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
let second_status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(first_status, PersonalityMigrationStatus::Applied);
|
||||
assert_eq!(second_status, PersonalityMigrationStatus::SkippedMarker);
|
||||
@@ -349,7 +332,8 @@ async fn no_marker_archived_sessions_sets_personality() -> io::Result<()> {
|
||||
let temp = TempDir::new()?;
|
||||
write_archived_session_with_user_event(temp.path()).await?;
|
||||
|
||||
let status = run_migration(temp.path(), &ConfigToml::default()).await?;
|
||||
let status =
|
||||
maybe_migrate_personality(temp.path(), &ConfigToml::default(), /*state_db*/ None).await?;
|
||||
|
||||
assert_eq!(status, PersonalityMigrationStatus::Applied);
|
||||
assert_eq!(
|
||||
|
||||
@@ -29,6 +29,7 @@ async fn build_prompt_input_includes_context_and_user_message() -> Result<()> {
|
||||
text: "hello from debug prompt".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
/*state_db*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -95,8 +95,7 @@ async fn emits_warning_when_resumed_model_differs() {
|
||||
let thread_manager = codex_core::test_support::thread_manager_with_models_provider(
|
||||
CodexAuth::from_api_key("test"),
|
||||
config.model_provider.clone(),
|
||||
)
|
||||
.await;
|
||||
);
|
||||
let auth_manager =
|
||||
codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("test"));
|
||||
|
||||
|
||||
@@ -34,8 +34,7 @@ async fn emits_warning_when_unstable_features_enabled_via_config() {
|
||||
let thread_manager = codex_core::test_support::thread_manager_with_models_provider(
|
||||
CodexAuth::from_api_key("test"),
|
||||
config.model_provider.clone(),
|
||||
)
|
||||
.await;
|
||||
);
|
||||
let auth_manager =
|
||||
codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("test"));
|
||||
|
||||
@@ -82,8 +81,7 @@ async fn suppresses_warning_when_configured() {
|
||||
let thread_manager = codex_core::test_support::thread_manager_with_models_provider(
|
||||
CodexAuth::from_api_key("test"),
|
||||
config.model_provider.clone(),
|
||||
)
|
||||
.await;
|
||||
);
|
||||
let auth_manager =
|
||||
codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("test"));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user