diff --git a/.github/workflows/local-ci.yml b/.github/workflows/local-ci.yml index 87e892eb56a0..0c5fab515dab 100644 --- a/.github/workflows/local-ci.yml +++ b/.github/workflows/local-ci.yml @@ -76,7 +76,12 @@ jobs: with: workspaces: codex-rs - name: Local crate tests - run: cargo test -p codex-local-cli -p codex-local-models -p codex-local-resume + # --lib: the codex-local bin include!s upstream cli/main.rs, so its bin + # target also compiles upstream's own cli tests (snapshots keyed to `codex`). + # Run only our bin test (the --no-daemon injection table) from it. + run: | + cargo test -p codex-local-cli -p codex-local-models -p codex-local-resume --lib + cargo test -p codex-local-cli --bin codex-local no_daemon - name: Hook tests (Tests lines in local/FORK_SURFACE.md) run: | set -euo pipefail diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 01d43eee2d45..2d6d224f4eaa 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3841,11 +3841,13 @@ name = "codex-local-cli" version = "0.162.1" dependencies = [ "anyhow", + "assert_matches", "chrono", "clap", "clap_complete", "codex-api", "codex-app-server", + "codex-app-server-client", "codex-app-server-daemon", "codex-app-server-protocol", "codex-app-server-test-client", @@ -3892,29 +3894,40 @@ dependencies = [ "codex-thread-store", "codex-tui", "codex-utils-absolute-path", + "codex-utils-cargo-bin", "codex-utils-cli", "codex-utils-path", "codex-utils-path-uri", "codex-utils-process", + "codex-utils-pty", "codex-utils-redacted-string", "codex-websocket-auth", "codex-windows-sandbox", "crossterm", "dirs", + "flate2", "futures", "http 1.4.0", + "insta", + "jsonwebtoken", "libc", "os_info", "owo-colors", + "predicates", + "pretty_assertions", "regex-lite", "reqwest 0.12.28", "serde", "serde_json", + "sha2 0.10.9", + "sqlx", "supports-color 3.0.2", "sys-locale", + "tar", "tempfile", "tikv-jemallocator", "tokio", + "tokio-tungstenite", "toml 0.9.11+spec-1.1.0", "toml_edit 0.24.0+spec-1.1.0", "tracing", @@ -3922,8 +3935,11 @@ dependencies = [ "tracing-subscriber", "unicode-segmentation", "url", + "vt100", "which 8.0.0", "windows-sys 0.61.2", + "wiremock", + "zstd", ] [[package]] diff --git a/codex-rs/local/cli/Cargo.toml b/codex-rs/local/cli/Cargo.toml index 452576c00a9b..65094830fa2c 100644 --- a/codex-rs/local/cli/Cargo.toml +++ b/codex-rs/local/cli/Cargo.toml @@ -18,8 +18,8 @@ doctest = false [[bin]] name = "codex-local" path = "src/main.rs" -# Upstream's in-file unit tests already run in the `codex-cli` package. -test = false +# Upstream's in-file unit tests already run in the `codex-cli` package; only run +# ours here (`cargo test -p codex-local-cli --bin codex-local no_daemon`). [dependencies] codex-cli = { path = "../../cli" } @@ -127,3 +127,19 @@ windows-sys = { workspace = true, features = [ [dev-dependencies] tempfile = { workspace = true } +assert_matches = { workspace = true } +codex-app-server-client = { workspace = true } +codex-utils-cargo-bin = { workspace = true } +codex-utils-pty = { workspace = true } +flate2 = { workspace = true } +insta = { workspace = true } +jsonwebtoken = { workspace = true } +predicates = { workspace = true } +pretty_assertions = { workspace = true } +sha2 = { workspace = true } +sqlx = { workspace = true } +tar = { workspace = true } +tokio-tungstenite = { workspace = true } +vt100 = { workspace = true } +wiremock = { workspace = true } +zstd = { workspace = true } diff --git a/codex-rs/local/cli/src/lib.rs b/codex-rs/local/cli/src/lib.rs index 19d210439bec..cd0ec697f7b1 100644 --- a/codex-rs/local/cli/src/lib.rs +++ b/codex-rs/local/cli/src/lib.rs @@ -80,7 +80,12 @@ pub fn prepare(args: Vec, env: EnvFn, user_home: Option) -> R return Ok(Action::SyncConfig); } let want_preflight = preflight::needs_preflight(&args, env); - let argv = apply_overrides(args, env)?; + let config_model = std::fs::read_to_string(homes.local.join("config.toml")) + .ok() + .and_then(|t| t.parse::().ok()) + .and_then(|c| c.get("model").and_then(|m| m.as_str()).map(str::to_string)) + .or_else(|| Some(DEFAULT_MODEL.to_string())); + let argv = apply_overrides(args, env, config_model)?; let preflight_root = want_preflight.then(|| effective_root(&argv, &homes, &root)); Ok(Action::Run { argv, diff --git a/codex-rs/local/cli/src/main.rs b/codex-rs/local/cli/src/main.rs index 9c45fc6dd2cf..16e0fbb18a29 100644 --- a/codex-rs/local/cli/src/main.rs +++ b/codex-rs/local/cli/src/main.rs @@ -6,6 +6,43 @@ include!("../../../cli/src/main.rs"); +/// codex-local registers its resume/model hooks in-process, so the TUI must run +/// in-process too; upstream's app-server daemon would drop them and needs a package +/// codex-local does not ship. Insert `--no-daemon` for plain TUI, `resume` and `fork` +/// unless the user chose `--remote` or already passed the flag. Anything that does +/// not parse (help, errors) is passed through unchanged. +fn inject_no_daemon(argv: Vec) -> Vec { + let Ok(cli) = MultitoolCli::try_parse_from(argv.clone()) else { + return argv; + }; + let (eligible, remote, sub_no_daemon) = match &cli.subcommand { + None => (true, false, false), + Some(Subcommand::Resume(c)) => ( + true, + c.remote.remote.is_some(), + c.config_overrides.0.no_daemon, + ), + Some(Subcommand::Fork(c)) => ( + true, + c.remote.remote.is_some(), + c.config_overrides.0.no_daemon, + ), + Some(_) => (false, false, false), + }; + if !eligible + || remote + || cli.remote.remote.is_some() + || cli.interactive.no_daemon + || sub_no_daemon + || argv.is_empty() + { + return argv; + } + let mut out = argv; + out.insert(1, "--no-daemon".into()); + out +} + fn main() -> anyhow::Result<()> { codex_build_info::initialize!(); let args: Vec = std::env::args_os().collect(); @@ -38,7 +75,49 @@ fn main() -> anyhow::Result<()> { codex_local_resume::register(); let remote_control_disabled = codex_app_server::take_remote_control_disabled_env(); arg0_dispatch_or_else(move |arg0_paths: Arg0DispatchPaths| async move { - Box::pin(cli_main(argv, arg0_paths, remote_control_disabled)).await?; + Box::pin(cli_main( + inject_no_daemon(argv), + arg0_paths, + remote_control_disabled, + )) + .await?; Ok(()) }) } + +#[cfg(test)] +mod no_daemon_tests { + use super::inject_no_daemon; + + #[test] + fn injection_decision_table() { + let cases: &[(&[&str], bool)] = &[ + (&["x"], true), + (&["x", "fix the bug"], true), + (&["x", "resume", "0192-abc"], true), + (&["x", "resume", "--last"], true), + (&["x", "fork", "--last"], true), + (&["x", "--no-daemon"], false), + (&["x", "resume", "--last", "--no-daemon"], false), + (&["x", "--remote", "ws://h:1"], false), + (&["x", "resume", "--last", "--remote", "ws://h:1"], false), + (&["x", "exec", "hi"], false), + (&["x", "app-server"], false), + (&["x", "app-server", "daemon", "start"], false), + (&["x", "--help"], false), + (&["x", "--bogus"], false), + (&["x", "--", "exec"], true), + ]; + for (args, injected) in cases { + let argv: Vec = args.iter().map(Into::into).collect(); + let out = inject_no_daemon(argv.clone()); + if *injected { + let mut want = argv; + want.insert(1, "--no-daemon".into()); + assert_eq!(out, want, "{args:?}"); + } else { + assert_eq!(out, argv, "{args:?}"); + } + } + } +} diff --git a/codex-rs/local/cli/src/overrides.rs b/codex-rs/local/cli/src/overrides.rs index 0b24c513a678..f9e0f9647553 100644 --- a/codex-rs/local/cli/src/overrides.rs +++ b/codex-rs/local/cli/src/overrides.rs @@ -16,8 +16,15 @@ pub(crate) const MAX_RETRIES: u64 = 2; /// Precedence: flags (`-m`, `--ollama-url`) > `CODEX_LOCAL_MODEL` > /// `CODEX_LOCAL_OLLAMA_URL` > `OLLAMA_HOST` > config.toml > built-ins. `-m` is left to /// upstream (it already wins), so the env model is injected only when no model flag is -/// present; config and built-ins need no injection. -pub fn apply_overrides(args: Vec, env: EnvFn) -> Result> { +/// present, else the configured model. Both the provider and the model are always +/// injected: upstream's TUI only overrides a resumed session's rollout provider/model +/// (e.g. a stock `openai`/`gpt-*` session) when a CLI layer sets them, and codex-local +/// must always continue on its own provider and model. +pub fn apply_overrides( + args: Vec, + env: EnvFn, + config_model: Option, +) -> Result> { let mut rest = Vec::with_capacity(args.len()); let mut flag_url: Option = None; let mut has_model_flag = false; @@ -58,7 +65,8 @@ pub fn apply_overrides(args: Vec, env: EnvFn) -> Result> out.push("-c".into()); out.push(kv.into()); }; - if !has_model_flag && let Some(m) = env("CODEX_LOCAL_MODEL") { + push_c(format!("model_provider=\"{PROVIDER_ID}\"")); + if !has_model_flag && let Some(m) = env("CODEX_LOCAL_MODEL").or(config_model) { push_c(format!("model={}", toml::Value::String(m))); } if let Some(root) = root { @@ -75,6 +83,8 @@ pub fn apply_overrides(args: Vec, env: EnvFn) -> Result> )); // Ollama fails the built-in server-side web_search tool; also covers old installs. push_c("web_search=\"disabled\"".to_string()); + // No app-server daemon (needs a package we don't ship; also skips our in-process hooks). + push_c("features.daemon_auto_start=false".to_string()); // `CODEX_LOCAL_MAX_AGENTS` beats the seeded `[agents] max_threads` in config.toml. if let Some(raw) = env("CODEX_LOCAL_MAX_AGENTS") { match raw.trim().parse::() { @@ -97,7 +107,7 @@ mod tests { .map(|(_, v)| v.to_string()) }; let a = args.iter().map(OsString::from).collect(); - apply_overrides(a, &f) + apply_overrides(a, &f, Some("cfgm".to_string())) .unwrap() .iter() .map(|s| s.to_string_lossy().into_owned()) @@ -107,6 +117,9 @@ mod tests { const RQ: &str = "model_providers.codex-local.request_max_retries=2"; const ST: &str = "model_providers.codex-local.stream_max_retries=2"; const WS: &str = "web_search=\"disabled\""; + const MP: &str = "model_provider=\"codex-local\""; + const MD: &str = "model=\"cfgm\""; + const DA: &str = "features.daemon_auto_start=false"; const URL_KEY: &str = "model_providers.codex-local.base_url"; #[test] @@ -114,11 +127,17 @@ mod tests { let want = [ "x", "-c", + MP, + "-c", + MD, + "-c", "model_providers.codex-local.request_max_retries=2", "-c", "model_providers.codex-local.stream_max_retries=2", "-c", "web_search=\"disabled\"", + "-c", + "features.daemon_auto_start=false", "exec", "hi", ]; @@ -129,12 +148,12 @@ mod tests { fn max_agents_env_valid_invalid_unset() { let k = "CODEX_LOCAL_MAX_AGENTS"; let ma = "agents.max_threads=8"; - assert_eq!(run(&["x", "exec"], &[(k, "8")])[8], ma); - assert_eq!(run(&["x", "exec"], &[(k, "8")]).len(), 10); + assert_eq!(run(&["x", "exec"], &[(k, "8")])[14], ma); + assert_eq!(run(&["x", "exec"], &[(k, "8")]).len(), 16); for bad in ["0", "-1", "abc", ""] { - assert_eq!(run(&["x", "exec"], &[(k, bad)]).len(), 8); + assert_eq!(run(&["x", "exec"], &[(k, bad)]).len(), 14); } - assert_eq!(run(&["x", "exec"], &[]).len(), 8); + assert_eq!(run(&["x", "exec"], &[]).len(), 14); } #[test] @@ -146,6 +165,10 @@ mod tests { [ "x", "-c", + MP, + "-c", + MD, + "-c", &want("http://f:3"), "-c", RQ, @@ -153,12 +176,14 @@ mod tests { ST, "-c", WS, + "-c", + DA, "exec" ] ); - assert_eq!(run(&["x", "exec"], &env)[2], want("http://h1:1")); + assert_eq!(run(&["x", "exec"], &env)[6], want("http://h1:1")); assert_eq!( - run(&["x", "exec"], &[("OLLAMA_HOST", "0.0.0.0")])[2], + run(&["x", "exec"], &[("OLLAMA_HOST", "0.0.0.0")])[6], want("http://127.0.0.1:11434") ); } @@ -166,17 +191,23 @@ mod tests { #[test] fn model_flag_beats_env_and_dd_is_respected() { let env = [("CODEX_LOCAL_MODEL", "envm")]; - assert_eq!(run(&["x", "exec"], &env)[2], "model=\"envm\""); + assert_eq!(run(&["x", "exec"], &env)[2], MP); + assert_eq!(run(&["x", "exec"], &env)[4], "model=\"envm\""); + assert_eq!(run(&["x", "exec"], &[])[4], "model=\"cfgm\""); assert_eq!( run(&["x", "-m", "flagm", "exec"], &env), [ "x", "-c", + MP, + "-c", "model_providers.codex-local.request_max_retries=2", "-c", "model_providers.codex-local.stream_max_retries=2", "-c", "web_search=\"disabled\"", + "-c", + "features.daemon_auto_start=false", "-m", "flagm", "exec" @@ -187,11 +218,17 @@ mod tests { [ "x", "-c", + MP, + "-c", + MD, + "-c", "model_providers.codex-local.request_max_retries=2", "-c", "model_providers.codex-local.stream_max_retries=2", "-c", "web_search=\"disabled\"", + "-c", + "features.daemon_auto_start=false", "exec", "--", "--ollama-url", diff --git a/codex-rs/local/cli/src/seed.rs b/codex-rs/local/cli/src/seed.rs index 9e7b690a8e01..d646e9147226 100644 --- a/codex-rs/local/cli/src/seed.rs +++ b/codex-rs/local/cli/src/seed.rs @@ -58,6 +58,8 @@ pub(crate) fn seed_toml(root: &str, stock_home: &str) -> Result { ("multi_agent", Value::Boolean(true)), // Gates the startup chatgpt.com featured/recommended-plugins fetches. ("plugins", Value::Boolean(false)), + // Run in-process; upstream's daemon needs a package codex-local lacks. + ("daemon_auto_start", Value::Boolean(false)), ]), ); // Ollama can't run the server-side `web_search` tool; use the local MCP server. @@ -93,6 +95,7 @@ mod tests { let v: toml::Value = toml::from_str(&body).unwrap(); assert_eq!(v["agents"]["max_threads"].as_integer(), Some(6)); assert_eq!(v["web_search"].as_str(), Some("disabled")); + assert_eq!(v["features"]["daemon_auto_start"].as_bool(), Some(false)); let ws = &v["mcp_servers"]["web-search"]; assert_eq!(ws["command"].as_str(), Some("uvx")); assert_eq!(ws["default_tools_approval_mode"].as_str(), Some("approve")); diff --git a/local/docs/plan-deltas.md b/local/docs/plan-deltas.md index 6226b94bf6a9..a6430c7a78de 100644 --- a/local/docs/plan-deltas.md +++ b/local/docs/plan-deltas.md @@ -14,3 +14,9 @@ Where the build differs from the approved plan. - **Agent threads:** seeded `[agents] max_threads` is now 6 (existing configs keep 4, never rewritten); override with `CODEX_LOCAL_MAX_AGENTS=` (positive int, beats config.toml, invalid warns and is ignored) or the `agents.max_threads` config key. - **UPSTREAM_SYNC_TOKEN:** the upstream-sync workflow needs this secret (PAT or App token with workflows scope) to push and open the PR whenever the merge touches `.github/workflows/**`; without it that run fails early, otherwise it falls back to GITHUB_TOKEN. - **Local web search:** Ollama fails the built-in server-side `web_search` tool, so config seeds `web_search = "disabled"` (also forced via `-c` on every run) and `[mcp_servers.web-search]` runs `uvx duckduckgo-mcp-server==0.7.0` (search + fetch_content, no key); `sync-config` keeps that entry; install/verify warn if `uvx` is missing. +- **No app-server daemon:** upstream auto-starts/discovers a daemon for the TUI that needs a full `codex-package.json` package (a bare binary fails with "no complete local package") and would bypass in-process registrations; codex-local injects `--no-daemon` (plain TUI/resume/fork, unless `--remote`) plus `-c features.daemon_auto_start=false`, and seeds that feature off; stock is untouched. + +## Cross-resume + +- Stock resumes a codex-local session with `codex resume -c model_provider=openai`; `codex exec resume ` needs no flag. +- codex-local always continues on its own provider and model: it injects `-c model_provider="codex-local"` and `-c model=` (unless `-m` is given), because upstream only overrides a resumed rollout's provider/model when a CLI layer sets them.