mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Enable worktrees by default and clarify local daemon errors (#44870)
## What changed - Mark `worktrees` stable and enable it by default, removing it from `/experimental`. Point users who explicitly disable it to their Codex configuration. - When a local daemon lacks `thread/backgroundTerminals/list`, block worktree creation and `/cd` with guidance to run `codex app-server daemon update` and restart Codex. ## Testing Add regression coverage for unsupported local daemon methods, unrelated errors, embedded servers, and empty background terminal lists. Update feature-gating tests to disable worktrees explicitly and remove worktrees from the experimental menu snapshot. GitOrigin-RevId: 3f21d0ebaa02da11a6aaa2e1ac615b4518fc98d8
This commit is contained in:
committed by
copyberry
parent
122d55cba8
commit
68bc5369ba
@@ -414,8 +414,16 @@ fn worktree_rejects_disabled_features_and_ignored_config_before_allocation() ->
|
||||
vec!["--ignore-user-config", "prompt"],
|
||||
"--worktree cannot be combined with --ignore-user-config",
|
||||
),
|
||||
("", vec!["prompt"], "--enable worktrees"),
|
||||
("", vec!["fork", "missing"], "--enable worktrees"),
|
||||
(
|
||||
"features.worktrees = false",
|
||||
vec!["prompt"],
|
||||
"--enable worktrees",
|
||||
),
|
||||
(
|
||||
"features.worktrees = false",
|
||||
vec!["fork", "missing"],
|
||||
"--enable worktrees",
|
||||
),
|
||||
(
|
||||
"features.worktrees = true",
|
||||
vec!["-c", "features.worktrees=false", "prompt"],
|
||||
|
||||
@@ -1254,12 +1254,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::Worktrees,
|
||||
key: "worktrees",
|
||||
stage: Stage::Experimental {
|
||||
name: "Worktrees",
|
||||
menu_description: "Create isolated Git worktrees and group sessions by repository.",
|
||||
announcement: "NEW: Worktrees can now be enabled from /experimental. Restart Codex after enabling it.",
|
||||
},
|
||||
default_enabled: false,
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::RespectSystemProxy,
|
||||
|
||||
@@ -10,6 +10,28 @@ use crate::history_cell::McpInventoryLoadingCell as LoadingCell;
|
||||
use codex_app_server_protocol::ThreadBackgroundTerminalsListParams;
|
||||
use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse as ListResponse;
|
||||
|
||||
pub(super) fn background_terminals_blocker(
|
||||
result: Result<ListResponse, TypedRequestError>,
|
||||
target: &AppServerTarget,
|
||||
) -> Option<&'static str> {
|
||||
match result {
|
||||
Ok(response) if response.data.is_empty() => None,
|
||||
Err(TypedRequestError::Server { source, .. })
|
||||
if matches!(target, AppServerTarget::LocalDaemon { .. })
|
||||
&& (source.code == -32601
|
||||
|| source.code == -32600
|
||||
&& source.message.contains("thread/backgroundTerminals/list")
|
||||
&& (source.message.contains("unknown variant")
|
||||
|| source.message.contains("unknown method"))) =>
|
||||
{
|
||||
Some(
|
||||
"The local Codex service cannot check background terminals. Run `codex app-server daemon update`, then restart Codex.",
|
||||
)
|
||||
}
|
||||
_ => Some("Active background terminals block /cd."),
|
||||
}
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) async fn start_managed_worktree(
|
||||
&mut self,
|
||||
@@ -19,7 +41,7 @@ impl App {
|
||||
) {
|
||||
if !self.config.features.enabled(Feature::Worktrees) {
|
||||
self.chat_widget.add_error_message(
|
||||
"Enable worktrees in /experimental to create a worktree.".to_string(),
|
||||
"Enable worktrees in your Codex configuration to create a worktree.".to_string(),
|
||||
);
|
||||
} else if self.config.active_project.is_untrusted() {
|
||||
self.chat_widget.add_error_message(
|
||||
@@ -123,8 +145,9 @@ impl App {
|
||||
.request_handle()
|
||||
.request_typed::<ListResponse>(request)
|
||||
.await;
|
||||
if !matches!(result, Ok(response) if response.data.is_empty()) {
|
||||
return self.working_directory_error("Active background terminals block /cd.");
|
||||
if let Some(message) = background_terminals_blocker(result, &self.app_server_target)
|
||||
{
|
||||
return self.working_directory_error(message);
|
||||
}
|
||||
}
|
||||
let setup = async {
|
||||
|
||||
@@ -69,6 +69,8 @@ mod transcript_composer;
|
||||
mod turn_submission;
|
||||
#[path = "tests/user_verification_routes_tests.rs"]
|
||||
mod user_verification_routes;
|
||||
#[path = "tests/worktree_background_terminals_tests.rs"]
|
||||
mod worktree_background_terminals_tests;
|
||||
|
||||
use super::*;
|
||||
use crate::app_backtrack::BacktrackSelection;
|
||||
|
||||
@@ -14,6 +14,7 @@ use pretty_assertions::assert_eq;
|
||||
#[tokio::test]
|
||||
async fn worktree_creation_event_requires_feature() -> Result<()> {
|
||||
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
|
||||
app.config.features.disable(Feature::Worktrees)?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?;
|
||||
while events.try_recv().is_ok() {}
|
||||
@@ -35,7 +36,7 @@ async fn worktree_creation_event_requires_feature() -> Result<()> {
|
||||
_ => None,
|
||||
})
|
||||
.expect("disabled feature message");
|
||||
insta::assert_snapshot!(message, @"■ Enable worktrees in /experimental to create a worktree.");
|
||||
insta::assert_snapshot!(message, @"■ Enable worktrees in your Codex configuration to create a worktree.");
|
||||
assert!(!app.config.codex_home.join("worktrees").exists());
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
use super::*;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn old_local_daemon_worktree_error_suggests_update() -> Result<()> {
|
||||
let target = AppServerTarget::LocalDaemon {
|
||||
endpoint: crate::RemoteAppServerEndpoint::UnixSocket {
|
||||
socket_path: AbsolutePathBuf::relative_to_current_dir("codex.sock")?,
|
||||
},
|
||||
};
|
||||
let missing_method = |code, message: &str| {
|
||||
Err(TypedRequestError::Server {
|
||||
method: "thread/backgroundTerminals/list".to_string(),
|
||||
source: JSONRPCErrorError {
|
||||
code,
|
||||
message: message.to_string(),
|
||||
data: None,
|
||||
},
|
||||
})
|
||||
};
|
||||
let message = managed_worktree_creation::background_terminals_blocker(
|
||||
missing_method(-32601, "method not found"),
|
||||
&target,
|
||||
)
|
||||
.expect("unsupported daemon must block worktree creation");
|
||||
let cell = crate::history_cell::new_error_event(message.to_string());
|
||||
insta::assert_snapshot!(
|
||||
lines_to_single_string(&cell.display_lines(/*width*/ 120)),
|
||||
@"■ The local Codex service cannot check background terminals. Run `codex app-server daemon update`, then restart Codex."
|
||||
);
|
||||
assert_eq!(
|
||||
managed_worktree_creation::background_terminals_blocker(
|
||||
missing_method(
|
||||
-32600,
|
||||
"Invalid request: unknown variant `thread/backgroundTerminals/list`"
|
||||
),
|
||||
&target,
|
||||
),
|
||||
Some(message),
|
||||
);
|
||||
assert_eq!(
|
||||
managed_worktree_creation::background_terminals_blocker(
|
||||
missing_method(-32600, "Invalid request: unknown variant `another/method`"),
|
||||
&target,
|
||||
),
|
||||
Some("Active background terminals block /cd."),
|
||||
);
|
||||
assert_eq!(
|
||||
managed_worktree_creation::background_terminals_blocker(
|
||||
missing_method(-32601, "method not found"),
|
||||
&AppServerTarget::Embedded,
|
||||
),
|
||||
Some("Active background terminals block /cd."),
|
||||
);
|
||||
assert_eq!(
|
||||
managed_worktree_creation::background_terminals_blocker(
|
||||
Ok(ThreadBackgroundTerminalsListResponse {
|
||||
data: Vec::new(),
|
||||
next_cursor: None,
|
||||
}),
|
||||
&target,
|
||||
),
|
||||
None,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -317,8 +317,11 @@ impl App {
|
||||
};
|
||||
let handle = app_server.request_handle();
|
||||
let result = handle.request_typed::<ListResponse>(request).await;
|
||||
if !matches!(result, Ok(response) if response.data.is_empty()) {
|
||||
return self.working_directory_error("Active background terminals block /cd.");
|
||||
if let Some(message) = super::managed_worktree_creation::background_terminals_blocker(
|
||||
result,
|
||||
&self.app_server_target,
|
||||
) {
|
||||
return self.working_directory_error(message);
|
||||
}
|
||||
}
|
||||
if is_new_worktree {
|
||||
|
||||
-2
@@ -11,8 +11,6 @@ expression: popup
|
||||
interactive website debugging and other inline
|
||||
JavaScript execution capabilities.
|
||||
[x] Shell tool Allow the model to run shell commands.
|
||||
[ ] Worktrees Create isolated Git worktrees and group sessions by
|
||||
repository.
|
||||
[ ] Voice conversations Talk with Codex using /voice.
|
||||
|
||||
Press space to select or enter to save
|
||||
|
||||
@@ -3038,7 +3038,6 @@ async fn apps_popup_for_not_installed_app_uses_install_only_selected_description
|
||||
async fn experimental_features_popup_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
let worktrees = Feature::Worktrees.stage();
|
||||
let voice = Feature::RealtimeConversation.stage();
|
||||
let features = vec![
|
||||
ExperimentalFeatureItem {
|
||||
@@ -3055,13 +3054,6 @@ async fn experimental_features_popup_snapshot() {
|
||||
description: "Allow the model to run shell commands.".to_string(),
|
||||
enabled: true,
|
||||
},
|
||||
ExperimentalFeatureItem {
|
||||
key: Feature::Worktrees.key().to_string(),
|
||||
writable: true,
|
||||
name: worktrees.experimental_menu_name().unwrap().to_string(),
|
||||
description: worktrees.experimental_menu_description().unwrap().to_string(),
|
||||
enabled: false,
|
||||
},
|
||||
ExperimentalFeatureItem {
|
||||
key: Feature::RealtimeConversation.key().to_string(),
|
||||
writable: true,
|
||||
|
||||
@@ -6,6 +6,7 @@ use pretty_assertions::assert_eq;
|
||||
#[tokio::test]
|
||||
async fn slash_new_and_fork_offer_checkout_choices_inside_local_git_repository() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::Worktrees, /*enabled*/ false);
|
||||
let checkout = tempdir().expect("temporary checkout");
|
||||
std::fs::create_dir(checkout.path().join(".git")).expect("git directory");
|
||||
std::fs::write(checkout.path().join(".git/HEAD"), "ref: refs/heads/main\n").expect("git HEAD");
|
||||
|
||||
@@ -82,7 +82,7 @@ impl ChatWidget {
|
||||
pub(super) fn show_managed_worktree_picker(&mut self) {
|
||||
if !self.config.features.enabled(Feature::Worktrees) {
|
||||
self.add_error_message(
|
||||
"Enable worktrees in /experimental to create a worktree.".to_string(),
|
||||
"Enable worktrees in your Codex configuration to create a worktree.".to_string(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -3265,6 +3265,9 @@ requires_openai_auth = {requires_openai_auth}
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
config
|
||||
.features
|
||||
.set_enabled(codex_features::Feature::Worktrees, /*enabled*/ false)?;
|
||||
let model_provider = config.model_provider_id.as_str();
|
||||
let project_thread_id = write_session_rollout(
|
||||
temp_dir.path(),
|
||||
|
||||
Reference in New Issue
Block a user