mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Reduce TUI legacy core dependencies (#26711)
## Why The TUI still reached through `app-server-client::legacy_core` for thread-name normalization and project-instruction filename details. In particular, checking the TUI's local filesystem for `/init` is incorrect for remote app-server sessions, where the server owns the working directory and instruction discovery. ## What changed - use the instruction source paths supplied by the app server to decide whether `/init` should avoid overwriting project instructions - keep the small thread-name normalization helper local to the TUI - remove the now-unused instruction filename constants, utility module, and other unused `legacy_core` re-exports - make status helper tests independent of concrete instruction filenames ## Verification - `just test -p codex-app-server-client` - `just test -p codex-tui slash_init_skips_when_project_instructions_are_loaded` - `just test -p codex-tui` ran 2,799 tests; 2,797 passed and two unrelated guardian feature-flag tests failed reproducibly in untouched code ### Manual test Started an app server over WebSocket with a remote workspace containing `AGENTS.md`, then connected the TUI using `--remote`. After confirming `thread/start` returned the file in `instructionSources`, deleted `AGENTS.md` and ran `/init` in the existing session. The TUI still reported that project instructions already existed and skipped `/init`. The trace contained no `turn/start` request, confirming the decision came from app-server session state rather than a new client-local filesystem check.
This commit is contained in:
committed by
GitHub
Unverified
parent
36bf63a5cf
commit
8e69d29521
@@ -70,13 +70,9 @@ pub use crate::remote::RemoteAppServerEndpoint;
|
||||
/// module exists so clients can remove a direct `codex-core` dependency
|
||||
/// while legacy startup/config paths are migrated to RPCs.
|
||||
pub mod legacy_core {
|
||||
pub use codex_core::DEFAULT_AGENTS_MD_FILENAME;
|
||||
pub use codex_core::LOCAL_AGENTS_MD_FILENAME;
|
||||
pub use codex_core::McpManager;
|
||||
pub use codex_core::check_execpolicy_for_warnings;
|
||||
pub use codex_core::format_exec_policy_error_with_source;
|
||||
pub use codex_core::grant_read_root_non_elevated;
|
||||
pub use codex_core::web_search_detail;
|
||||
|
||||
pub mod config {
|
||||
pub use codex_core::config::*;
|
||||
@@ -86,10 +82,6 @@ pub mod legacy_core {
|
||||
}
|
||||
}
|
||||
|
||||
pub mod connectors {
|
||||
pub use codex_core::connectors::*;
|
||||
}
|
||||
|
||||
pub mod otel_init {
|
||||
pub use codex_core::otel_init::*;
|
||||
}
|
||||
@@ -98,22 +90,10 @@ pub mod legacy_core {
|
||||
pub use codex_core::personality_migration::*;
|
||||
}
|
||||
|
||||
pub mod review_format {
|
||||
pub use codex_core::review_format::*;
|
||||
}
|
||||
|
||||
pub mod review_prompts {
|
||||
pub use codex_core::review_prompts::*;
|
||||
}
|
||||
|
||||
pub mod test_support {
|
||||
pub use codex_core::test_support::*;
|
||||
}
|
||||
|
||||
pub mod util {
|
||||
pub use codex_core::util::*;
|
||||
}
|
||||
|
||||
pub mod windows_sandbox {
|
||||
pub use codex_core::windows_sandbox::*;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
Generate a file named AGENTS.md that serves as a contributor guide for this repository.
|
||||
Before writing, check whether AGENTS.md already exists in the current working directory. If it does, do not overwrite or modify it.
|
||||
Your goal is to produce a clear, concise, and well-structured document with descriptive headings and actionable explanations for each section.
|
||||
Follow the outline below, but adapt as needed — add sections if relevant, and omit those that do not apply to this project.
|
||||
|
||||
|
||||
@@ -58,7 +58,6 @@ use crate::bottom_pane::TerminalTitleItem;
|
||||
use crate::bottom_pane::TerminalTitleSetupView;
|
||||
use crate::diff_model::FileChange;
|
||||
use crate::git_action_directives::parse_assistant_markdown;
|
||||
use crate::legacy_core::DEFAULT_AGENTS_MD_FILENAME;
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::legacy_core::config::Constrained;
|
||||
use crate::legacy_core::config::ConstraintResult;
|
||||
@@ -257,6 +256,11 @@ fn queued_message_edit_hint_binding(
|
||||
.or_else(|| bindings.first().copied())
|
||||
}
|
||||
|
||||
fn normalize_thread_name(name: &str) -> Option<String> {
|
||||
let trimmed = name.trim();
|
||||
(!trimmed.is_empty()).then(|| trimmed.to_string())
|
||||
}
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event::ExitMode;
|
||||
use crate::app_event::PermissionProfileSelection;
|
||||
|
||||
@@ -302,7 +302,7 @@ impl ChatWidget {
|
||||
/*initial_text*/ existing_name.unwrap_or_default().to_string(),
|
||||
/*context_label*/ None,
|
||||
Box::new(move |name: String| {
|
||||
let Some(name) = crate::legacy_core::util::normalize_thread_name(&name) else {
|
||||
let Some(name) = normalize_thread_name(&name) else {
|
||||
tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_error_event("Thread name cannot be empty.".to_string()),
|
||||
)));
|
||||
|
||||
@@ -213,14 +213,6 @@ impl ChatWidget {
|
||||
.send(AppEvent::OpenDesktopThread { thread_id });
|
||||
}
|
||||
SlashCommand::Init => {
|
||||
let init_target = self.config.cwd.join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
if init_target.exists() {
|
||||
let message = format!(
|
||||
"{DEFAULT_AGENTS_MD_FILENAME} already exists here. Skipping /init to avoid overwriting it."
|
||||
);
|
||||
self.add_info_message(message, /*hint*/ None);
|
||||
return;
|
||||
}
|
||||
const INIT_PROMPT: &str = include_str!("../../prompt_for_init_command.md");
|
||||
self.submit_user_message(INIT_PROMPT.to_string().into());
|
||||
}
|
||||
@@ -662,7 +654,7 @@ impl ChatWidget {
|
||||
}
|
||||
self.session_telemetry
|
||||
.counter("codex.thread.rename", /*inc*/ 1, &[]);
|
||||
let Some(name) = crate::legacy_core::util::normalize_thread_name(&args) else {
|
||||
let Some(name) = normalize_thread_name(&args) else {
|
||||
self.add_error_message("Thread name cannot be empty.".to_string());
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -565,35 +565,14 @@ async fn ctrl_d_with_modal_open_does_not_quit() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_init_skips_when_project_doc_exists() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let tempdir = tempdir().unwrap();
|
||||
let existing_path = tempdir.path().join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
std::fs::write(&existing_path, "existing instructions").unwrap();
|
||||
chat.config.cwd = tempdir.path().to_path_buf().abs();
|
||||
async fn slash_init_does_not_depend_on_loaded_instruction_sources() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.instruction_source_paths = vec![chat.config.cwd.join("project-instructions.md")];
|
||||
|
||||
submit_composer_text(&mut chat, "/init");
|
||||
|
||||
match op_rx.try_recv() {
|
||||
Err(TryRecvError::Empty) => {}
|
||||
other => panic!("expected no Codex op to be sent, got {other:?}"),
|
||||
}
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected one info message");
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains(DEFAULT_AGENTS_MD_FILENAME),
|
||||
"info message should mention the existing file: {rendered:?}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("Skipping /init"),
|
||||
"info message should explain why /init was skipped: {rendered:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(existing_path).unwrap(),
|
||||
"existing instructions"
|
||||
);
|
||||
assert_eq!(chat.input_queue.queued_user_messages.len(), 1);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
assert_eq!(recall_latest_after_clearing(&mut chat), "/init");
|
||||
}
|
||||
|
||||
|
||||
@@ -185,8 +185,6 @@ fn title_case(s: &str) -> String {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::legacy_core::DEFAULT_AGENTS_MD_FILENAME;
|
||||
use crate::legacy_core::LOCAL_AGENTS_MD_FILENAME;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -227,7 +225,7 @@ mod tests {
|
||||
async fn compose_agents_summary_includes_global_agents_path() {
|
||||
let codex_home = TempDir::new().expect("temp codex home");
|
||||
let cwd = TempDir::new().expect("temp cwd");
|
||||
let global_agents_path = codex_home.path().join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
let global_agents_path = codex_home.path().join("global.md");
|
||||
let config = test_config(&codex_home, &cwd).await;
|
||||
|
||||
assert_eq!(
|
||||
@@ -240,7 +238,7 @@ mod tests {
|
||||
async fn compose_agents_summary_names_global_agents_override() {
|
||||
let codex_home = TempDir::new().expect("temp codex home");
|
||||
let cwd = TempDir::new().expect("temp cwd");
|
||||
let override_path = codex_home.path().join(LOCAL_AGENTS_MD_FILENAME);
|
||||
let override_path = codex_home.path().join("override.md");
|
||||
let config = test_config(&codex_home, &cwd).await;
|
||||
|
||||
assert_eq!(
|
||||
@@ -253,8 +251,8 @@ mod tests {
|
||||
async fn compose_agents_summary_orders_global_before_project_agents() {
|
||||
let codex_home = TempDir::new().expect("temp codex home");
|
||||
let cwd = TempDir::new().expect("temp cwd");
|
||||
let global_agents_path = codex_home.path().join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
let project_agents_path = cwd.path().join(DEFAULT_AGENTS_MD_FILENAME);
|
||||
let global_agents_path = codex_home.path().join("global.md");
|
||||
let project_agents_path = cwd.path().join("project.md");
|
||||
let config = test_config(&codex_home, &cwd).await;
|
||||
|
||||
let summary = compose_agents_summary(
|
||||
@@ -270,7 +268,7 @@ mod tests {
|
||||
Some(format_directory_display(&global_agents_path, /*max_width*/ None).as_str())
|
||||
);
|
||||
let project_path = paths.next().expect("project agents path");
|
||||
assert!(project_path.ends_with(DEFAULT_AGENTS_MD_FILENAME));
|
||||
assert!(project_path.ends_with("project.md"));
|
||||
assert_eq!(paths.next(), None);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user