openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-20 12:47:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Allow worktree sessions to use an existing local daemon (#46498)

Browse Source 
## Why

`--worktree` and command-line worktree feature overrides previously excluded sessions from using the local daemon, even though worktree allocation is client-owned and thread requests already forward the feature.

## What changed

- Allow `--worktree` and boolean `features.worktrees` overrides to remain eligible for daemon connections, while preserving exclusions for other configuration overrides and `--no-daemon`.
- Skip daemon auto-start for `--worktree` launches.

## Testing

Add daemon eligibility tests for worktree options and overrides. Run the existing worktree startup and fork test scenarios against both embedded and daemon backends, checking ownership before the first turn and confirming daemon connections through `/status`.

GitOrigin-RevId: b3f4782e83d8e20974fb4a831442cae013d58e56
This commit is contained in:
Eric Traut
2026-09-18 02:54:39 +00:00
committed by copyberry
parent c0e1b78253
commit e497393552
4 changed files with 164 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

144
codex-rs/cli/tests/worktree.rs
View File

@@ -9,6 +9,7 @@ use std::io::Write as _;
use std::path::Path;
use std::process::Command;
use std::time::Duration;
#[cfg(not(unix))]
use tempfile::TempDir;
use wiremock::Mock;
use wiremock::MockServer;
@@ -133,6 +134,23 @@ async fn rejected_start(
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn interactive_worktree_start_and_fork_bind_owner_before_turn() -> anyhow::Result<()> {
worktree_start_and_fork("embedded").await
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[cfg_attr(
windows,
ignore = "requires a non-elevated terminal with daemon breakaway support"
)]
async fn daemon_worktree_start_and_fork_bind_owner_before_turn() -> anyhow::Result<()> {
worktree_start_and_fork("daemon").await
}
async fn worktree_start_and_fork(backend: &str) -> anyhow::Result<()> {
// Leave room for the local control socket on Unix platforms with short sun_path limits.
#[cfg(unix)]
let root = tempfile::Builder::new().tempdir_in("/tmp")?;
#[cfg(not(unix))]
let root = TempDir::new()?;
let root = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(root.path())?
.canonicalize()?
@@ -165,8 +183,9 @@ async fn interactive_worktree_start_and_fork_bind_owner_before_turn() -> anyhow:
r#"
cli_auth_credentials_store = "file"
chatgpt_base_url = "{}/source/backend-api"
features.worktrees = true
analytics.enabled = false
check_for_update_on_startup = false
features.daemon_auto_start = true
model_provider = "local"
model = "test-model"
sandbox_mode = "workspace-write"
@@ -229,6 +248,38 @@ trust_level = "trusted"
] {
env.remove(key);
}
// Keep the managed package local; no installer or updater participates in this test.
struct StopDaemon(Command);
impl Drop for StopDaemon {
fn drop(&mut self) {
let _ = self.0.output();
}
}
let _daemon = if backend == "daemon" {
let bin = home.join("packages/app-server-daemon/current/bin");
fs::create_dir_all(&bin)?;
let managed = bin.join(if cfg!(windows) { "codex.exe" } else { "codex" });
fs::hard_link(&program, &managed).or_else(|_| fs::copy(&program, &managed).map(|_| ()))?;
fs::create_dir(home.join("app-server-daemon"))?;
fs::write(
home.join("app-server-daemon/settings.json"),
r#"{"shutdownGraceSeconds":0,"updater":{"autoUpdateEnabled":false}}"#,
)?;
let mut daemon_stop = Command::new(&program);
daemon_stop
.envs(&env)
.args(["app-server", "daemon", "stop"]);
Some(StopDaemon(daemon_stop))
} else {
None
};
// Every launch, including rejection cases, exercises the same CLI feature opt-in.
let worktree_args: Vec<String> = vec![
"--worktree".into(),
"--enable".into(),
"worktrees".into(),
"--no-alt-screen".into(),
];
let mut owner = None;
let mut previous: Vec<String> = Vec::new();
for (fork, explicit_cd, analytics, auth_failure) in [
@@ -307,7 +358,19 @@ trust_level = "trusted"
})
.mount(&server)
.await;
let mut args = vec!["--worktree".to_owned(), "--no-alt-screen".to_owned()];
if backend == "daemon" && previous.is_empty() {
// Policy mocks must be ready before the daemon loads its startup configuration.
let started = Command::new(&program)
.envs(&env)
.args(["app-server", "daemon", "start"])
.output()?;
anyhow::ensure!(
started.status.success(),
"daemon startup failed: {}",
String::from_utf8_lossy(&started.stderr)
);
}
let mut args = worktree_args.clone();
if fork {
args.extend([
"--add-dir".into(),
@@ -396,13 +459,12 @@ trust_level = "trusted"
assert!(output.contains("The checkout was kept"), "{output}");
continue;
}
let renamed = if !fork && matches!(observed, Ok(Ok(_))) {
let renamed = if !fork && let Ok(Ok((_, _, metadata))) = &observed {
tokio::time::timeout(Duration::from_secs(/*secs*/ 10), async {
session
.writer_sender()
.send(b"/rename managed-source\r".to_vec())
.await?;
let (_, _, metadata) = observed.as_ref().unwrap().as_ref().unwrap();
let thread_id = codex_protocol::ThreadId::from_string(
metadata["ownerThreadId"].as_str().context("owner id")?,
)?;
@@ -431,6 +493,25 @@ trust_level = "trusted"
} else {
Ok(())
};
if backend == "daemon" && !analytics && matches!(observed, Ok(Ok(_))) {
session.writer_sender().send(b"/status\r".to_vec()).await?;
tokio::time::timeout(Duration::from_secs(/*secs*/ 10), async {
while !output.contains("app-server-control.sock") {
let bytes = stdout
.recv()
.await
.context("TUI exited before daemon status")?;
output.push_str(&String::from_utf8_lossy(&bytes));
let replies = queries.feed(&bytes);
if !replies.is_empty() {
session.writer_sender().send(replies).await?;
}
}
Ok::<_, anyhow::Error>(())
})
.await
.with_context(|| format!("daemon status timed out: {output}"))??;
}
if analytics {
tokio::time::timeout(Duration::from_secs(/*secs*/ 10), metric_rx.recv())
.await?
@@ -446,7 +527,7 @@ trust_level = "trusted"
);
}
if matches!(startup_result, Ok(Ok(()))) && renamed.is_ok() {
if !fork {
if !fork && backend == "embedded" {
for (input, expected) in [
("/daemon\r", "Install latest public stable"),
("\r", "Update and exit"),
@@ -500,12 +581,14 @@ trust_level = "trusted"
.flat_map(|resource| resource["scopeMetrics"].as_array().into_iter().flatten())
.flat_map(|scope| scope["metrics"].as_array().into_iter().flatten())
.collect::<Vec<_>>();
let update = exported
.iter()
.find(|metric| metric["name"] == "codex.daemon.update")
.context("handoff metric")?;
let update_point = &update["sum"]["dataPoints"][0];
assert_eq!(update_point["asInt"], 1);
if backend == "embedded" {
let update = exported
.iter()
.find(|metric| metric["name"] == "codex.daemon.update")
.context("handoff metric")?;
let update_point = &update["sum"]["dataPoints"][0];
assert_eq!(update_point["asInt"], 1);
}
let point = exported
.iter()
.filter(|metric| metric["name"] == "codex.tui.start")
@@ -525,20 +608,25 @@ trust_level = "trusted"
))
})
.collect::<anyhow::Result<HashMap<_, _>>>()?;
assert_eq!(
tags,
HashMap::from([
("app_server_mode", "in_process"),
("terminal_name", "unknown"),
("multiplexer", "none"),
("daemon_selection_reason", "incompatible_option"),
("daemon_auto_start", "disabled"),
("auto_update", "enabled"),
("auto_update_setting", "default"),
("update_interval_setting", "default"),
("shutdown_grace_setting", "default"),
])
);
let mut expected_tags = HashMap::from([
("app_server_mode", "in_process"),
("terminal_name", "unknown"),
("multiplexer", "none"),
("daemon_selection_reason", "incompatible_option"),
("daemon_auto_start", "enabled"),
("auto_update", "enabled"),
("auto_update_setting", "default"),
("update_interval_setting", "default"),
("shutdown_grace_setting", "default"),
]);
if backend == "daemon" {
expected_tags.extend([
("auto_update", "disabled"),
("auto_update_setting", "configured"),
("shutdown_grace_setting", "configured"),
]);
}
assert_eq!(tags, expected_tags);
} else {
assert!(metrics.is_empty(), "analytics disabled");
}
@@ -636,6 +724,8 @@ trust_level = "trusted"
&program,
&[
"--worktree".into(),
"--enable".into(),
"worktrees".into(),
"--no-alt-screen".into(),
"fork".into(),
next_owner.clone(),
@@ -669,6 +759,8 @@ trust_level = "trusted"
&program,
&[
"--worktree".into(),
"--enable".into(),
"worktrees".into(),
"--no-alt-screen".into(),
"--cd".into(),
source.join(".codex").display().to_string(),
@@ -730,6 +822,8 @@ trust_level = "trusted"
&program,
&[
"--worktree".into(),
"--enable".into(),
"worktrees".into(),
"--no-alt-screen".into(),
"--cd".into(),
source.display().to_string(),

14
codex-rs/tui/src/daemon_startup.rs
View File

@@ -14,8 +14,6 @@ pub(super) fn exclusion(
) -> Option<&'static str> {
if cli.no_daemon {
Some("--no-daemon")
} else if cli.shared.worktree {
Some("--worktree")
} else if cli.oss {
Some("--oss")
} else if workload_identity_selected {
@@ -42,7 +40,17 @@ pub(super) fn config_exclusion(
strict_config: bool,
bypass_hook_trust: bool,
) -> Option<&'static str> {
if !cli_kv_overrides.is_empty() {
// Worktree allocation is client-owned, and thread requests already forward this feature.
if !cli_kv_overrides
.iter()
.all(|(key, value)| match key.as_str() {
"features.worktrees" => value.is_bool(),
"features" => value.as_table().is_some_and(|features| {
features.len() == 1 && features.get("worktrees").is_some_and(toml::Value::is_bool)
}),
_ => false,
})
{
Some("command-line configuration overrides (-c, --enable, --disable, or --search)")
} else if !loader_overrides_are_default(loader_overrides) {
Some("custom configuration loader")

34
codex-rs/tui/src/daemon_startup_tests.rs
View File

@@ -5,6 +5,37 @@ use crate::legacy_core::config::ConfigBuilder;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
#[test]
fn worktree_feature_override_allows_daemon_without_allowing_other_features() {
for (raw, eligible) in [
("features.worktrees=true", true),
("features.worktrees=false", true),
("features={worktrees=true}", true),
("features={worktrees=true,shell_tool=false}", false),
("features.shell_tool=false", false),
("features.worktrees.enabled=true", false),
("features={}", false),
("model='test'", false),
] {
let overrides = codex_utils_cli::CliConfigOverrides {
raw_overrides: vec![raw.to_string()],
}
.parse_overrides()
.unwrap();
assert_eq!(
daemon_startup::config_exclusion(
&overrides,
&LoaderOverrides::default(),
/*strict_config*/ false,
/*bypass_hook_trust*/ false,
)
.is_none(),
eligible,
"{raw}"
);
}
}
#[test]
fn daemon_launch_telemetry_records_once_on_connection_or_early_return() {
for connected in [false, true] {
@@ -122,7 +153,8 @@ fn daemon_eligibility_preserves_launch_options_and_explains_exclusions() {
use clap::Parser;
for (args, expected) in [
("--no-daemon", Some("--no-daemon")),
("--worktree", Some("--worktree")),
("--worktree", None),
("--worktree --no-daemon", Some("--no-daemon")),
("--oss", Some("--oss")),
("--profile test", Some("--profile")),
("--strict-config", Some("--strict-config")),

1
codex-rs/tui/src/startup_orchestration.rs
View File

@@ -400,6 +400,7 @@ pub(super) async fn run_main_inner(
))
.await?;
let auto_start_daemon = config.features.enabled(Feature::DaemonAutoStart)
&& !cli.shared.worktree
&& !cli.agents_overview
&& !cli.no_daemon
&& !app_server_target.uses_remote_workspace();