[4 of 4] tui: route startup and onboarding config writes through app server#22916
Closed
etraut-openai wants to merge 4 commits into
Closed
[4 of 4] tui: route startup and onboarding config writes through app server#22916etraut-openai wants to merge 4 commits into
etraut-openai wants to merge 4 commits into
Conversation
This was referenced May 15, 2026
fb52e01 to
8cac60f
Compare
d8375c8 to
d5f9063
Compare
8cac60f to
6ea4edd
Compare
etraut-openai
added a commit
that referenced
this pull request
May 16, 2026
## Why The TUI can run against a remote app server, but several high-traffic settings still persisted by editing the local config file. That sends remote sessions' preference writes to the wrong machine and lets local disk state drift from the app-server-owned config. This is **[1 of 4]** in a stacked series that moves TUI-owned config mutations onto app-server APIs. ## What changed - Added a small TUI helper for typed app-server config writes. - Routed primary interactive preference writes through `config/batchWrite`. - Preserved existing profile scoping for settings that already support `profiles.<profile>.*` overrides. ## Config keys affected - `model` - `model_reasoning_effort` - `personality` - `service_tier` - `plan_mode_reasoning_effort` - `approvals_reviewer` - `notice.fast_default_opt_out` - Profile-scoped equivalents under `profiles.<profile>.*` ## Suggested manual validation - Connect the TUI to a remote app server, change `model` and `model_reasoning_effort`, reconnect, and confirm the remote config retained both values while the local `config.toml` did not change. - Change `personality`, `plan_mode_reasoning_effort`, and the explicit auto-review selection, then reconnect and confirm those choices persist through the app server. - Clear the service tier back to default and confirm `service_tier` is cleared while `notice.fast_default_opt_out = true` is persisted remotely. - Repeat one setting change with an active profile and confirm the write lands under `profiles.<profile>.*`. ## Stack 1. [#22913](#22913) `[1 of 4]` primary settings writes 2. [#22914](#22914) `[2 of 4]` app and skill enablement 3. [#22915](#22915) `[3 of 4]` feature and memory toggles 4. [#22916](#22916) `[4 of 4]` startup and onboarding bookkeeping
etraut-openai
added a commit
that referenced
this pull request
May 19, 2026
## Why App and skill toggles are user config mutations too. When the TUI is attached to a remote app server, writing those toggles into the local `config.toml` makes the UI report success without updating the server that actually owns the session. This is **[2 of 4]** in a stacked series that moves TUI-owned config mutations onto app-server APIs. ## What changed - Routed app enable/disable persistence through app-server config batch writes. - Routed skill enable/disable persistence through `skills/config/write`. - Avoided refreshing local config from disk after these writes when the TUI is connected to a remote app server. ## Config keys affected - `apps.<app_id>.enabled` - `apps.<app_id>.disabled_reason` - `[[skills.config]]` entries keyed by `path`, with `enabled = false` used for persisted disables ## Suggested manual validation - Connect the TUI to a remote app server, disable an app, reconnect, and confirm the app remains disabled from remote config rather than local disk state. - Re-enable the same app and confirm both `apps.<app_id>.enabled` and `apps.<app_id>.disabled_reason` are cleared remotely. - Disable a skill in the manage-skills UI and confirm a remote `[[skills.config]]` disable entry appears. - Re-enable that skill and confirm the disable entry is removed and the effective enabled state updates without relying on local config reloads. ## Stack 1. [#22913](#22913) `[1 of 4]` primary settings writes 2. [#22914](#22914) `[2 of 4]` app and skill enablement 3. [#22915](#22915) `[3 of 4]` feature and memory toggles 4. [#22916](#22916) `[4 of 4]` startup and onboarding bookkeeping
f0e5a0e to
61beab3
Compare
etraut-openai
added a commit
that referenced
this pull request
May 21, 2026
…2915) ## Why Experimental feature toggles and memory settings can update several related config values in one interaction. Keeping those writes local in a remote TUI session is especially dangerous because the UI can diverge from the app-server config while also leaving behind partially stale supporting keys. This is **[3 of 4]** in a stacked series that moves TUI-owned config mutations onto app-server APIs. ## What changed - Routed feature flag persistence through app-server batch writes, including the supporting reviewer and permission updates used by guardian approval. - Routed Windows sandbox mode persistence and legacy Windows feature cleanup through app-server writes. - Routed memory settings through app-server batch writes and updated the TUI tests to exercise the embedded app-server path. ## Config keys affected - `features.<feature_key>` - `profiles.<profile>.features.<feature_key>` - `approval_policy` - `sandbox_mode` - `approvals_reviewer` - `windows.sandbox` - `features.experimental_windows_sandbox` - `features.elevated_windows_sandbox` - `features.enable_experimental_windows_sandbox` - Profile-scoped Windows legacy feature variants under `profiles.<profile>.features.*` - `memories.use_memories` - `memories.generate_memories` - Profile-scoped memory variants under `profiles.<profile>.memories.*` ## Suggested manual validation - Connect the TUI to a remote app server, toggle guardian approval on and off, and confirm the remote config updates `features.guardian_approval`, reviewer state, approval policy, and sandbox mode coherently. - Toggle a default-false experimental feature at the root level, disable it again, and confirm the key clears instead of lingering as an unnecessary explicit `false`. - Change memory settings and confirm the remote config updates both memory keys while the running TUI reflects the new state. - On Windows, switch sandbox mode through the TUI and confirm `windows.sandbox` is updated while the legacy Windows feature keys are cleared. ## Stack 1. [#22913](#22913) `[1 of 4]` primary settings writes 2. [#22914](#22914) `[2 of 4]` app and skill enablement 3. [#22915](#22915) `[3 of 4]` feature and memory toggles 4. [#22916](#22916) `[4 of 4]` startup and onboarding bookkeeping
Base automatically changed from
etraut/tui-config-app-server-3-feature-toggles
to
main
May 21, 2026 23:03
2ec355f to
91d4b25
Compare
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91d4b257b2
ℹ️ 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".
…-server-4-startup-bookkeeping # Conflicts: # codex-rs/tui/src/app/config_persistence.rs
This was referenced May 23, 2026
etraut-openai
added a commit
that referenced
this pull request
May 25, 2026
## Summary The TUI `/mcp` inventory flow should reflect the app server’s MCP status response. It was also joining those results with the TUI process’s local `config.mcp_servers`, which can diverge once MCP state is owned by a remote app server and cause stale local command, URL, status, or empty-state details to render. This change removes the local config join from the app-server-backed inventory renderer. The TUI now renders directly from the existing `mcpServerStatus/list` payload and treats an empty status response as the empty MCP inventory state. ## Known limitation The existing `mcpServerStatus/list` payload does not include disabled-state or disabled-reason fields. To preserve the current app-server API, this PR does not try to infer that state from client-local config. If remote `/mcp` needs to show disabled/reason details again, that should come from app-server-owned status data in a follow-up. Related to #22914, #22915, and #22916.
manoelcalixto
added a commit
to Electivus/electivus-codex
that referenced
this pull request
May 26, 2026
* Report app-server version in codex doctor (openai#24311) ## Why We are seeing cases where users have an old background app-server still running. `codex doctor` already reports background server state, but without the running app-server version it is harder to diagnose behaviors that depend on the daemon build. ## What changed - Reused the app-server daemon's passive initialize probe through a narrow `probe_app_server_version` helper. - Updated the `codex doctor` Background Server section to report `app-server version: <version>` when the socket is reachable. - Preserved the not-running OK behavior and report `app-server version: unavailable (<short error>)` when a socket exists but the passive probe fails. * tui: label compact rate-limit percentages (openai#24314) ## Summary The compact TUI status line already renders rate-limit percentages as remaining capacity, but the text did not say so. That made high-usage red indicators ambiguous because values like `weekly 6%` could be read as either used or remaining. This PR labels the compact rate-limit values explicitly as `left` across the status line, terminal title, and setup previews. Addresses openai#24274 * Show remote connection details in /status (openai#24420) ## Summary Fixes openai#24411. `/status` currently has no way to show when the TUI is talking to Codex through a remote transport. That makes embedded local sessions, local daemon sessions, and true remote sessions look the same, and it hides the remote server version when debugging connection-specific behavior. This PR adds a single `Remote` row for non-embedded connections only. The row shows the sanitized connection address and a dimmed version parenthetical, preserving the existing status output for embedded local sessions. <img width="791" height="144" alt="image" src="https://github.com/user-attachments/assets/529d7940-1c45-4586-8b06-f20a1f04b771" /> ## Verification - Manually validated when connecting remotely (either implicitly to local daemon or explicitly) * Respect hook trust bypass during TUI startup (openai#24317) Fixes openai#24093. ## Why `--dangerously-bypass-hook-trust` is a supported CLI flag intended for headless or automated runs where enabled hooks should be allowed to run without requiring persisted trust. In the TUI, startup hook review still opened whenever hooks looked untrusted, so a launch using the bypass could block on the interactive "Hooks need review" prompt. The tricky case is persistent app-server resume: a resume may attach to an already-running thread, where resume config overrides are ignored. In that path, hiding the startup review would be wrong because the existing hook engine may still filter untrusted hooks. ## What Changed - Startup hook review now skips the prompt only when hook trust bypass is actually safe for that launch. - The TUI forwards `bypass_hook_trust` through the app-server request config for fresh thread start/resume/fork paths, and the app-server applies it as a runtime-only `ConfigOverrides` value rather than treating it like a `config.toml` setting. - Persistent app-server resumes keep the startup review prompt so users still have a chance to trust hooks when the running thread cannot receive the bypass override. ## Verification - Added focused coverage for startup hook review with and without `bypass_hook_trust`. - Extended existing TUI/app-server config override tests to cover forwarding and applying `bypass_hook_trust`. * TUI config cleanup: oss_provider (openai#24254) ## Summary Manual provider selection during `codex --oss` startup was still persisting `oss_provider` through the legacy local `config.toml` writer. That bypasses the app-server-owned config mutation path used by the TUI, so this routes the write through the app server config API instead. The net behavior is intentionally narrow: only an interactive picker selection is persisted. Auto-detected single-running-provider startup and explicit `--local-provider` startup remain ephemeral, so merely having one backend running does not make that provider sticky for future runs. ## What Changed - Removed the TUI picker’s direct dependency on `set_default_oss_provider`. - Had `oss_selection` report whether the returned provider came from the interactive picker. - Carried only manually selected providers into startup persistence. - Wrote `oss_provider` via `config/batchWrite` once the app server session is available. - Logged a warning and continued startup if the app-server config write fails. ## Verification Manually smoke-tested the real `codex-tui` binary with a temporary `CODEX_HOME`, pseudo-terminal input, and a fake LM Studio HTTP server: - Interactive picker selection persisted `oss_provider = "lmstudio"`. - Non-picker `--local-provider lmstudio` startup did not persist `oss_provider`. * TUI config cleanup: trusted projects (openai#24255) ## Why TUI onboarding trusted-project persistence should go through the same app-server config write path as other config mutations. Writing `config.toml` directly from the trust widget bypasses that layer and can let onboarding proceed even when the trust decision was not actually persisted. ## What changed - Added a TUI config helper that writes the existing project trust structure through `config/batchWrite`. - Persists trust decisions as `projects.<project>.trust_level = "trusted"` using the existing project trust key helper. - Changed the trust directory widget to only record the user selection; onboarding performs the app-server write before reporting success. - Keeps the user on the trust screen and shows an error if app-server persistence fails. ## Verification - `cargo test -p codex-tui --lib trust_persistence_failure_keeps_trust_step_in_progress` - `cargo test -p codex-tui --lib trusted_project_edit_targets_project_trust_level` - Manual: built the local `codex-cli`, accepted the trust prompt in a temp project, confirmed `projects.<project>.trust_level = "trusted"`, and simulated an unwritable config to verify onboarding stays on the trust screen without writing trust. * TUI config cleanup: MCP inventory (openai#24265) ## Summary The TUI `/mcp` inventory flow should reflect the app server’s MCP status response. It was also joining those results with the TUI process’s local `config.mcp_servers`, which can diverge once MCP state is owned by a remote app server and cause stale local command, URL, status, or empty-state details to render. This change removes the local config join from the app-server-backed inventory renderer. The TUI now renders directly from the existing `mcpServerStatus/list` payload and treats an empty status response as the empty MCP inventory state. ## Known limitation The existing `mcpServerStatus/list` payload does not include disabled-state or disabled-reason fields. To preserve the current app-server API, this PR does not try to infer that state from client-local config. If remote `/mcp` needs to show disabled/reason details again, that should come from app-server-owned status data in a follow-up. Related to openai#22914, openai#22915, and openai#22916. * Add doctor thread inventory audit (openai#24305) ## Why Users have been reporting missing sessions in the app. The app server thread listing is backed by the SQLite state DB, but the durable source of truth for a thread still exists on disk as rollout JSONL. When the state DB is incomplete, doctor should be able to show the mismatch directly instead of leaving users with a generic state health result. ## What changed This adds a `threads` doctor check that compares active and archived rollout files under `CODEX_HOME` with rows in the SQLite `threads` table. The check reports missing rollout rows, stale DB rows, archive flag mismatches, duplicate rollout thread IDs, duplicate DB paths, source/provider summaries, and bounded samples of affected rollout paths. It also adds a read-only state audit helper in `codex-rs/state` so doctor can inspect thread rows without creating, migrating, or repairing the database. ## Sample output ```text ⚠ threads rollout files are missing from the state DB default model provider openai rollout DB active files 3910 rollout DB archived files 2037 rollout DB scan errors 0 rollout DB malformed file names 0 rollout DB scan cap reached false rollout DB rows 5499 rollout DB active rows 3462 rollout DB archived rows 2037 rollout DB missing active rows 448 rollout DB missing archived rows 0 rollout DB stale rows 0 rollout DB archive mismatches 0 rollout DB duplicate rollout thread ids 0 rollout DB duplicate DB paths 0 rollout DB model providers openai=5359, lmstudio=35, mock_provider=33, lite_llm=26, proxy=26, ollama=15, lms=4, local-usage-limit=1 rollout DB sources vscode=2587, cli=1494, subagent:thread_spawn=577, subagent:other=502, exec=281, subagent:memory_consolidation=46, subagent:review=9, unknown=3 rollout DB missing active sample ~/.codex/sessions/2026/0…857e-a923c712e066.jsonl rollout DB missing active sample ~/.codex/sessions/2025/0…877a-766dff25c68d.jsonl rollout DB missing active sample ~/.codex/sessions/2025/0…a8b1-7bbadc836f6e.jsonl rollout DB missing active sample ~/.codex/sessions/2025/0…a218-e6197f3f62f8.jsonl rollout DB missing active sample ~/.codex/sessions/2025/0…9011-7e30784f9932.jsonl ``` * fix(tui): improve markdown table column allocation (openai#24346) ## Why Markdown tables with a long path-heavy column could allocate almost all available width to that column and collapse neighboring prose columns to only a few characters. In rollout summaries this made `Unit` and `What It Adds` difficult to read, even though the long `Files` values were the content best suited to wrapping. The affected example also specified `Files` as right aligned in its markdown delimiter (`---:`). This change preserves that requested alignment while improving how width is distributed. | Before | After | |---|---| | <img width="1709" height="764" alt="image" src="https://github.com/user-attachments/assets/932ab21c-b72d-48a2-9aad-b69da87a0968" /> | <img width="1711" height="855" alt="image" src="https://github.com/user-attachments/assets/4028bd20-2228-4c2f-be8a-1866325b7f62" /> | ## What Changed - Classify table columns as narrative, token-heavy, or compact during width allocation. - Shrink token-heavy path and URL columns before shrinking narrative prose, while preserving compact counts and short labels longest. - Use readable soft floors for narrative and token-heavy content before falling back to tighter layouts. - Add snapshot coverage for a rollout-shaped table containing right-aligned file paths and prose columns. ## How to Test 1. Render a markdown table with `Unit`, right-aligned `Files`, `Adds`, `Removes`, and `What It Adds` columns at a constrained terminal width. 2. Put long repository paths in `Files` and sentence-length content in `Unit` and `What It Adds`. 3. Confirm that `Files` remains right aligned but wraps before the narrative columns become unreadable. 4. Confirm that the compact numeric columns remain easy to scan. Targeted tests: - `just test -p codex-tui markdown_render` Validation note: `just test -p codex-tui` was also attempted and reached two existing unrelated failures in `app::tests::update_feature_flags_disabling_guardian_*`; the markdown rendering regression test passes in the targeted run. * fix(tui): improve multiline markdown list readability (openai#24351) ## Why Numbered Markdown findings become hard to scan when long items visually run together or when wrapped explanatory paragraphs lose their list indentation. This is especially visible in review output: the next number can look attached to the previous finding, and paragraph continuation rows can jump back toward the left margin instead of staying grouped beneath their item. <table><tr><td> <center>Before</center> <img width="1718" height="836" alt="CleanShot 2026-05-24 at 14 00 49" src="https://github.com/user-attachments/assets/f1ee0023-50fa-4f81-a641-ae08b17b99bd" /> </td></tr> <tr><td> <center>After</center> <img width="1714" height="906" alt="image" src="https://github.com/user-attachments/assets/b123a5e0-a232-47bf-96d5-c935295f7c0a" /> </td></tr> </table> ## What Changed - Insert a blank separator before a sibling list item when the previous item occupies more than one rendered line. - Preserve compact rendering for lists whose sibling items each render on one line. - Preserve list-body leading whitespace when transient streamed assistant rows require another wrapping pass for history display, so wrapped paragraphs stay aligned beneath their item. - Share the existing leading-whitespace prefix logic used by history insertion instead of introducing a second indentation rule. - Keep streamed Markdown output aligned with completed rendering and add snapshots for findings-style spacing and streamed paragraph indentation. ## How to Test 1. Start Codex from this branch and open the recorded repro session `019e563f-7d58-7ff2-8ec7-828f20fa61ca`. 2. Inspect the numbered `Findings` list whose items contain explanatory paragraphs. 3. Confirm each multiline finding is separated from the next numbered finding by one blank line. 4. Confirm wrapped rows of each indented paragraph remain aligned beneath the finding body, rather than returning to the left edge. 5. Render a short one-line numbered or unordered list and confirm its items remain compact without added blank rows. Targeted tests: - `just test -p codex-tui history_cell insert_history markdown_render markdown_stream streaming::controller` - `just argument-comment-lint-from-source -p codex-tui` ## Related Work PR openai#24346 changes Markdown table column allocation in parallel. This PR is intentionally limited to list-item readability and history wrapping; both branches touch `codex-rs/tui/src/markdown_render.rs`, so a small merge conflict may need resolution depending on merge order. * fix(tui): prevent macos stderr from corrupting composer (openai#24459) ## Why Fixes openai#17139. On macOS, runtime diagnostics such as `MallocStackLogging` messages can be written directly to process stderr while the inline TUI owns the terminal. Those bytes paint into the same viewport as the composer without passing through the renderer or composer state, making diagnostic output appear to leak into the input area. ## What Changed - Add a macOS terminal stderr guard while the inline TUI owns the viewport. - Restore stderr when Codex returns terminal ownership for external interactive programs, suspend/resume, panic handling, and normal shutdown. - Add an fd-level regression test that verifies output is suppressed only while terminal ownership is held and restored at each handoff boundary. ## How to Test 1. On macOS, launch the interactive TUI and leave the composer visible. 2. Exercise the workflow that triggers an allocator/runtime stderr diagnostic during an active session, as reported in openai#17139. 3. Confirm the diagnostic no longer overwrites the active composer region. 4. Suspend or exit the TUI and confirm subsequent terminal stderr output remains visible. The platform diagnostic is environment-dependent, so the deterministic regression check is the new fd-lifecycle test in `tui::terminal_stderr::tests::suppresses_stderr_only_while_terminal_is_owned`. Targeted validation: - `just argument-comment-lint-from-source -p codex-tui` passed. - `just test -p codex-tui` exercised and passed the new stderr-guard regression test. The full invocation currently fails in two unrelated guardian-policy tests, `update_feature_flags_disabling_guardian_clears_review_policy_and_restores_default` and `update_feature_flags_disabling_guardian_clears_manual_review_policy_without_history`, which reproduce when rerun in isolation. * fix(process-hardening): preserve macos malloc diagnostics (openai#24479) ## Summary Follow-up to openai#24459 and partial behavioral revert of `a71fc47` / openai#16699. - Stop removing `MallocStackLogging*` and `MallocLogFile*` from macOS pre-main hardening. - Remove documentation that claims Codex suppresses those allocator diagnostic controls. - Retain the shared `remove_env_vars_with_prefix` refactor and existing `LD_` / `DYLD_` hardening. ## Why openai#24459 fixes the composer-corruption problem at the terminal stderr boundary while preserving redirected stderr. With that guard in place, stripping macOS malloc diagnostic settings is unnecessary and can hide diagnostics intentionally enabled by callers. ## Validation - `just fmt` - `just test -p codex-process-hardening` - `just argument-comment-lint-from-source -p codex-process-hardening` - `git diff --check` --------- Co-authored-by: Eric Traut <etraut@openai.com> Co-authored-by: Felipe Coury <felipe.coury@openai.com> Co-authored-by: electivus-codex-bot <actions@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The remaining local writes happen during startup, onboarding, and prompt bookkeeping. They are less common than interactive settings changes, but they are still real config mutations and should follow the same app-server ownership model instead of reaching around it.
This is [4 of 4] in a stacked series that moves TUI-owned config mutations onto app-server APIs.
What changed
config/batchWrite.Config keys affected
oss_providerprojects.<project_key>.trust_leveltui.model_availability_nux.<model_slug>notice.hide_full_access_warningnotice.hide_world_writable_warningnotice.hide_rate_limit_model_nudgenotice.model_migrations.<from_model>notice.external_config_migration_prompts.homenotice.external_config_migration_prompts.home_last_prompted_atnotice.external_config_migration_prompts.projects.<project_key>notice.external_config_migration_prompts.project_last_prompted_at.<project_key>Suggested manual validation
--osswhen no OSS provider is configured, pick a provider manually, and confirmoss_provideris persisted through the app-server path.projects.<project_key>.trust_level.Stack
[1 of 4]primary settings writes[2 of 4]app and skill enablement[3 of 4]feature and memory toggles[4 of 4]startup and onboarding bookkeeping