Skip to content

code-mode: move to hosted mode by default #31500

Open
cconger wants to merge 7 commits into
mainfrom
cconger/code-mode-host-default
Open

code-mode: move to hosted mode by default #31500
cconger wants to merge 7 commits into
mainfrom
cconger/code-mode-host-default

Conversation

@cconger

@cconger cconger commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Promote code_mode_host to stable and enable it by default.
  • Preserve features.code_mode_host = false as an opt-out to the in-process runtime.
  • Run core code-mode tests through the standalone host.
  • Keep explicit coverage for missing-host failures.

@cconger cconger requested a review from a team as a code owner July 7, 2026 23:37

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad91ea2053

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/BUILD.bazel Outdated
crate_name = "codex_core",
extra_binaries = [
"//codex-rs/bwrap:bwrap",
"//codex-rs/code-mode-host:code-mode-host",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use the code-mode host binary label

When Bazel wires extra_binaries, it derives the CARGO_BIN_EXE_* variable from the label name; this label is the library target code-mode-host, while the test helper looks up cargo_bin("codex-code-mode-host"). In Bazel core tests the new hosted default therefore will not receive a CARGO_BIN_EXE_codex-code-mode-host runfile path, so code-mode tests fall back to a non-existent sibling of the test binary and fail to spawn the host; point this at //codex-rs/code-mode-host:codex-code-mode-host instead.

AGENTS.md reference: AGENTS.md:L216-L220

Useful? React with 👍 / 👎.

let code_mode_host_program = self
.code_mode_host_program
.take()
.or_else(|| codex_utils_cargo_bin::cargo_bin("codex-code-mode-host").ok());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Build or disable the host in Cargo tests

Under the documented local path just test -p codex-core, Cargo does not build a binary from the separate codex-code-mode-host package, so this .ok() path is usually None; because ThreadManager::new already selected the process-owned provider from the now-default code_mode_host, code-mode tests then try to spawn codex-code-mode-host next to the test executable and report tool errors instead of exercising the runtime. The builder needs to ensure a real host binary is available for scoped Cargo runs, or explicitly disable CodeModeHost before constructing the manager when it is not.

AGENTS.md reference: AGENTS.md:L64-L68

Useful? React with 👍 / 👎.

stage: Stage::UnderDevelopment,
default_enabled: false,
stage: Stage::Stable,
default_enabled: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Honor per-thread code-mode-host opt-outs

With code_mode_host now defaulting on, a long-lived app-server/TUI process constructs ThreadManager once from the startup config, but each thread/start later reloads and passes its own config overrides to start_thread_with_options; if that per-thread config sets features.code_mode_host=false while using code mode, the session still receives the already-selected process-owned provider and cannot opt back into the in-process runtime. This makes the advertised opt-out ineffective for app-server config overrides unless the provider choice is moved to thread/session creation or rebuilt from the thread config.

AGENTS.md reference: AGENTS.md:L102-L110

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants