feat(tui): migrate TUI to in-process app-server#14018
feat(tui): migrate TUI to in-process app-server#14018fcoury wants to merge 46 commits intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates the TUI (and related CLI agent wiring) to use the new in-process app-server client facade, reducing duplicated “app-server-like” logic in each surface and aligning in-process behavior with the JSON-RPC app-server semantics.
Changes:
- Plumbs
Arg0DispatchPaths/CloudRequirementsLoaderinto the TUI startup path and agent initialization to support in-process app-server startup. - Replaces the TUI agent thread wiring with an
InProcessAppServerClient-backed loop that routesOp↔ app-server typed requests/events while keeping the legacy event bridge. - Adds defensive UI behavior to prevent duplicate interactive requests from being queued and to dismiss stale modals on interrupted turns.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| codex-rs/tui/src/lib.rs | Threads arg0_paths + cloud_requirements into ratatui app startup. |
| codex-rs/tui/src/chatwidget.rs | Introduces InProcessAgentContext and updates widget constructors/interrupt handling for the new agent model. |
| codex-rs/tui/src/chatwidget/agent.rs | New in-process app-server agent loop: request routing, event translation, local history helpers, and bootstrap logic. |
| codex-rs/tui/src/chatwidget/tests.rs | Updates test init plumbing and adds coverage for reopen/shutdown + interrupted-turn modal behavior. |
| codex-rs/tui/src/bottom_pane/mod.rs | Adds dismiss_all_views() to clear modal stack on interruption. |
| codex-rs/tui/src/bottom_pane/request_user_input/mod.rs | Prevents duplicate RequestUserInput requests from being queued twice; adds test. |
| codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs | Prevents duplicate MCP elicitation requests from being queued twice; adds test. |
| codex-rs/tui/src/bottom_pane/approval_overlay.rs | Prevents duplicate approval requests from being queued twice; adds test. |
| codex-rs/tui/src/app.rs | Stores ambient in-process context and improves handling of existing primary threads during resume/fork flows. |
| codex-rs/tui/Cargo.toml | Adds dependency on codex-app-server-client. |
| codex-rs/exec/src/lib.rs | Adds bootstrap helpers for session-configured handling and tightens thread-id validation in server-request handling. |
| codex-rs/app-server/src/app_server_tracing.rs | Ensures typed in-process spans record a request id via client_request_id(). |
| codex-rs/app-server-client/src/lib.rs | Adds ClientSurface → SessionSource mapping and centralizes overloaded error code constant. |
| codex-rs/app-server-protocol/src/protocol/common.rs | Minor formatting cleanup. |
| codex-rs/Cargo.lock | Locks new codex-app-server-client dependency for TUI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
76338d6 to
7c78c1f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c78c1f730
ℹ️ 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".
7c78c1f to
6f21e7b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 040239d412
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4f3b6e6 to
481cc32
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 481cc32aef
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ae7d4c1fb
ℹ️ 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".
9ae7d4c to
5b053ca
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af132093ff
ℹ️ 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".
Track current turn ids and pending interactive request cleanup per thread inside the in-process agent loop. Child-thread shutdown now unsubscribes only that thread instead of tearing down the shared app-server session. This prevents primary turn completion from invalidating child prompts, keeps sub-agent shutdown from killing the whole session, and avoids interrupts targeting a turn from the wrong thread after view switches.
Translate app-server child `thread/closed` notifications into the existing routed shutdown path and keep child turn bookkeeping scoped to the completed thread. This keeps the agent picker and selected-thread state in sync when app-server-managed child threads finish, and prevents stale child turn ids or pending prompts from surviving past completion.
Replace one-shot shared auth mutation with scoped external auth override guards so nested in-process clients restore only their own layer on shutdown. This keeps the latest active refresher and forced ChatGPT workspace bound to the shared `AuthManager` until the owning client exits, instead of reverting to stale state when an older client drops.
Keep forced ChatGPT workspace resolution layered when nested external auth overrides omit their own workspace id. This prevents a newer in-process client from temporarily clearing an inherited workspace pin while it is active, which keeps refreshes on the shared `AuthManager` bound to the expected workspace.
Advance to the next queued `request_user_input` prompt after interrupting the active one, instead of dismissing the entire overlay and stranding other threads behind it. Also render requested permissions from the original profile rather than expanded picker choices so macOS permission prompts only show the permission the server actually requested.
When switching away from an app-server-backed thread, remove the thread from the shared manager and submit `Op::Shutdown` directly to the removed thread. This avoids racing the in-process app-server loop against `remove_thread()`, which could leave the old thread running after a session switch because shutdown never reached the backing thread.
Treat primary `thread/closed` notifications like shutdown completion in the in-process agent loop. This makes externally closed primary sessions tear down through the normal shutdown-finalization path instead of leaving the old client alive on a dead transport.
Backfill in-process listener attachment for threads that already exist in a shared `ThreadManager` when the client initializes. This keeps preloaded or resumed threads observable through the in-process app-server path instead of only auto-subscribing future `thread_created` events.
Skip stale queued `request_user_input` prompts from the interrupted turn instead of advancing into requests that core has already cleared. Also let the permissions picker consume plain `o` for search in the permissions view so cross-thread shortcuts do not steal normal text input.
Capture the originating `thread_id` when interactive prompts are deferred behind the stream controller, and replay them against that stored thread instead of the currently selected widget thread. This keeps approvals and user-input prompts routed to the thread that originally emitted them after view switches, and updates the chatwidget tests to cover the real inbound event paths.
Route `request_user_input` confirmation cells through a thread-scoped app event instead of the global `InsertHistoryCell` path. This stores local confirmation cells with the originating thread so background-thread answers do not write into the visible transcript and still replay when switching back to that thread.
Only the primary live thread should reuse the in-process app-server op sender when switching views. This keeps selected secondary threads on the direct `CodexThread` path until their local response events are routed per thread, which avoids applying skill, prompt, and history UI updates to the primary thread by mistake.
Write exec and permission approval decisions through `InsertThreadHistoryCell` so inactive threads keep the same local confirmation history as the active thread. This preserves deny and partial-grant context after switching threads and updates the TUI tests to accept the thread-scoped history path.
…y in order Send `Op::Shutdown` directly to the primary core thread when the active session is app-server-backed, instead of routing that exit through thread-scoped app-server ops that only unsubscribe the thread stream. Replace the thread-history replay side channel with an ordered replay timeline so local-only history cells interleave with buffered events. This keeps rollback behavior aligned with transcript reconstruction during thread replay.
Use the live `AuthManager` for local ChatGPT token refreshes in app-server-backed sessions and route that path through the configured external auth refresher. This avoids returning stale cached tokens after unauthorized responses and keeps refresh state aligned with the shared thread manager. Treat refreshed ChatGPT account-id mismatches as hard failures during local refresh handling. This preserves the existing re-authentication behavior instead of silently switching a running session to a different account.
Make bottom-pane interrupt cleanup aware of the interrupted thread so turn-scoped approvals and user-input overlays from other live threads stay queued. The active thread still clears its own turn-scoped views, while unscoped views keep the historical interrupt behavior. Thread-owned bottom-pane overlays now report their `thread_id`, and the interrupt path filters on that ownership when dismissing views. This prevents one thread interruption from silently removing actionable prompts belonging to another thread.
Carry the original event `id` through legacy notification decoding when app-server includes one in the wrapper payload. TUI replay and deduplication logic uses non-empty event ids for user-message handling, so dropping them can reintroduce duplicate transcript entries. Keep the existing empty-string fallback for legacy payloads that do not include an event id, and cover both shapes in decoder tests.
Drop the leftover inherited-next-turn permission hooks that no longer exist in `SessionState`, and reconcile the TUI approval code with the upstream permissions picker shape after the rebase. Restore `MessageProcessorArgs.thread_manager` so the in-process `app-server` still reuses the shared `ThreadManager` expected by the TUI and `app-server-client`, keeping the rebased branch compiling without reviving the old branch-specific permissions path.
Adjust the in-process app-server tests to use the current `ThreadManager::new` signature after the rebase cleanup. This keeps the shared-manager test coverage compiling without reviving older constructor arguments that `core` no longer accepts.
Deduplicate in-process permission approvals by `(thread_id, call_id)` and keep the permissions overlay behavior aligned with the current TUI flow, including updated replay and test coverage for the `scope` field. Route shared in-process auth overrides through the existing `AuthManager` guard stack so nested clients unwind correctly, and drop an unused TUI test helper to keep the crate warning-free.
ea36017 to
f5664bc
Compare
Update the rebased test helpers to match the current `MessageProcessorArgs` and `ThreadManager` interfaces after the latest upstream changes. Tighten the app-server tracing wait predicate so it waits for spans from the request trace, and keep `Cargo.lock` in sync with the rebased crate metadata used by this branch.
Format the rebased app-server tracing test the same way CI's `cargo fmt` expects so the format job passes again. Broaden the macOS seatbelt denial matcher to accept the current bash stderr variant that prefixes denied writes with `line 1:` while keeping the existing sandbox-apply fallback.
Problem
The TUI talked to agent threads via a direct
CodexThreadhandle: it created threads throughThreadManager, polled events withnext_event(), and forwarded user operations by callingCodexThread::submit(). This by-passed the app-server protocol layer, meaning the TUI could not take advantage of the in-process app-server's session management, config-reload logic, or typed interactive-request routing. It also meant the TUI had no handling for theRequestPermissionstool event, so permission-escalation prompts were silentlydropped.
This is the TUI half of the migration originally planned in #13636. The foundation work (
codex-app-server-clientfacade,in_processruntime host,MessageProcessorextraction) already landed; this PR ports the TUI surface onto it and adds the multi-thread event routing, auth-override nesting, and sharedThreadManagersupport needed to run child agent sessions in-process. Exec migration is handled separately.Mental model
There are three layers between the TUI and
MessageProcessor:codex_app_server::in_process— low-level runtime host. Owns theMessageProcessor, outbound routing task, and bounded typed channels. Performs theinitialize/initializedhandshake before returning anInProcessClientHandle.codex-app-server-client— shared facade crate. Wraps the low-level handle behind a worker task with asyncmpscchannels, adds surface identity (ClientSurface→SessionSource), typed request helpers, backpressure-aware event forwarding, and bounded shutdown with abort fallback.TUI surface (
chatwidget/agent.rs) — consumes events from the facade and maps them into the TUI's existingEventMsg/AppEventvocabulary. No longer performs initialize, session-source selection, or shutdown directly.After this change the TUI's primary session runs through an in-process
InProcessAppServerClientrather than a rawCodexThread. The agent task boots a full app-server runtime, sendsthread/startandturn/startJSON-RPC requests, and translates incomingServerRequest/ServerNotification/ legacy-notification events back intoEventMsg.For resumed or forked sessions the TUI still receives a
CodexThreadfromThreadManager, but registers that thread directly (attach_existing_primary_thread) without spawning a redundantnext_event()listener, because the app-server's own event stream already delivers thoseevents.
A new
ApprovalRequest::Permissionsvariant andMultiSelectPickerwidget let the user toggle individual permission grants (network, file-system roots, macOS entitlements) before confirming.Auth overrides use a stack-based RAII model (
ExternalAuthOverrideGuard) so nested in-process sessions can temporarily redirect token refresh without corrupting the sharedAuthManager.Non-goals
handle_thread_created→register_live_thread.active_session_events_via_app_server = true). There is no feature flag to fall back to the old direct-thread path.codex_protocol::Eventbridge is preserved; removing it is a separate follow-up.Migration boundaries
Actions kept local in TUI
Handled locally (no app-server RPC needed):
AddToHistory,GetHistoryEntryRequest,ListCustomPromptsForwarded directly to
CodexThread(bypassing app-server, functional):ReloadUserConfig,OverrideTurnContext,DropMemories,UpdateMemories,RunUserShellCommand,ListMcpToolsDeferred (emit local warning, not functional):
Undo— emits "temporarily unavailable in in-process local-only mode"Shutdownis intentionally not a dedicated RPC — it maps tothread/unsubscribeplus local completion handling.Notifications staying on the legacy event bridge
These notifications are not on the typed app-server path:
warning,backgroundEvent,turn/streamErrorthread/undo/started,thread/undo/completed,thread/shutdown/completedmcpServer/startup/updated,mcpServer/startup/completedskills/updatedThree server notifications use the typed app-server path today:
ServerNotification::ItemStarted(file-change caching)ServerNotification::ServerRequestResolved(MCP elicitation cleanup)ServerNotification::ThreadClosed(turn-state cleanup, primary-thread shutdown trigger)Everything else flows through the legacy
LegacyNotification→EventMsgbridge, including realtime event consumption (realtime requests were migrated, but event consumption was not).Other non-migrated pieces
account/chatgptAuthTokens/refreshremains a local server-request adapter rather than a deeper app-server-owned flow.Tradeoffs
agent.rsInProcessAgentContextcloned into everyChatWidgetAppand the agent task, at the cost of cloningArg0DispatchPaths, CLI overrides, andCloudRequirementsLoaderon each session creation.serde_json::Valueresults rather than directly typed structs, becauseMessageProcessorproduces that shape internally. This trades deserialization cost for behavioral parity with socket transports.-32001(overloaded) instead of blocking or growing unbounded. A slow consumer can cause approval flows to be rejected rather than queued — intentional, since hanging approvals are worse than explicit rejection.same_identityApprovalOverlay,McpServerElicitationOverlay, andRequestUserInputOverlay.preserve_on_turn_interruptClientSurface::Tuimaps toSessionSource::CliThreadManager+ direct listeners. This avoids re-serializing rollout history through app-server but adds routing complexity inApp.PendingServerRequeststracks bothCommandExecutionRequestApproval(V1) andExecCommandApproval(V2). Preserves backward compatibility during the migration window at the cost of enum duplication.Architecture
Key routing paths:
spawn_agentstarts the in-process client, sendsthread/start, and enters the event loop.process_in_process_commandmaps eachOpvariant to a JSON-RPC client request.ServerRequestdispatch maps typed approval requests back toEventMsgevents and tracks pending request IDs so responses can be resolved.attach_existing_primary_threadregisters a thread created byThreadManager(resume/fork) into the channel infrastructure without starting a competingnext_event()listener.Key type additions
ExternalAuthOverrideGuardcore/auth.rsThreadEventStoretui/app.rsThreadEventChanneltui/app.rsThreadScopedOptui/chatwidget/agent.rsOptagged with targetthread_idfor cross-thread routingPendingServerRequeststui/chatwidget/agent.rs(thread_id, item_id)PendingInteractiveReplayStatetui/app/pending_interactive_replay.rsApprovalRequest::Permissionstui/bottom_pane/approval_overlay.rsMultiSelectPickerThreadInteractiveRequesttui/app.rsContracts
TurnComplete,TurnAborted,ShutdownCompleteuse blockingsend()on the in-process channel — they are never dropped under backpressure.push_external_auth_overridereturns an RAII guard; dropping the guard removes that entry from the stack. ANoneworkspace ID inherits from the nearest outer override.mpsc::Receiver<Event>is active in theselect!loop at a time. Switching threads transfers ownership back toThreadEventChannel.receiver.PendingInteractiveReplayState::should_replay_snapshot_event()suppresses resolved approvals/input when switching threads;TurnComplete/TurnAbortedclear all turn-scoped pending state.request_user_inputprompts are answered in arrival order per(thread_id, turn_id). The overlay always resolves the oldest queuedRequestId.Files modified
codex-rs/tui/src/chatwidget/agent.rs— migrated to useInProcessAppServerClientfacade (+3700 lines).codex-rs/tui/src/chatwidget.rs— addedInProcessAgentContext,RequestPermissionsevent handling, turn-interrupt overlay cleanup.codex-rs/tui/src/app.rs— plumbedArg0DispatchPaths/CloudRequirementsLoader, addedregister_live_thread/attach_existing_primary_thread,RequestPermissionsapproval routing, multi-thread event channel infrastructure.codex-rs/tui/src/app/pending_interactive_replay.rs— replay-state tracking for thread-switch snapshot filtering.codex-rs/tui/src/bottom_pane/approval_overlay.rs— newPermissionsvariant,MultiSelectPickerintegration, duplicate-request dedup.codex-rs/tui/src/bottom_pane/mod.rs—dismiss_turn_scoped_views.codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs— dedup +preserve_on_turn_interrupt.codex-rs/tui/src/bottom_pane/request_user_input/mod.rs— dedup, queued multi-request architecture.codex-rs/tui/src/chatwidget/interrupts.rs—RequestPermissionsvariant, thread-scoped interrupt tracking.codex-rs/tui/src/bottom_pane/bottom_pane_view.rs—preserve_on_turn_interrupttrait method.codex-rs/tui/src/lib.rs— passarg0_pathsandcloud_requirementsthrough toApp.codex-rs/core/src/auth.rs— stack-basedExternalAuthOverrideGuardwith RAII cleanup, workspace inheritance.codex-rs/core/src/thread_manager.rs— publicauth_manager()accessor, publicsend_op.codex-rs/app-server/src/in_process.rs— in-process runtime with sharedThreadManagersupport, existing-thread listener attachment.codex-rs/app-server/src/message_processor.rs— optionalThreadManagerreuse, RAII-based auth override guard.codex-rs/app-server-client/src/lib.rs—thread_managerfield for sharedThreadManagerpass-through.Observability
app_server.requestspans withrpc.transport = "in-process", making them distinguishable from stdio/websocket spans in traces.tracing::warnwhen events are dropped or server requests are rejected due to backpressure.send_warning_event/send_error_eventsurface operational problems as TUI-visible warnings.CODEX_TUI_RECORD_SESSION=1logs inboundAppEventand outboundOpas timestamped JSONL, includingthread_idfor multi-thread debugging.InProcessServerEvent::Lagged { skipped }propagated to client for transport health monitoring.Tests
pending_interactive_replay.rs: 5 new tests coveringRequestPermissionslifecycle (enqueue, resolve, turn-abort cleanup, pending-approval flag).approval_overlay.rs: 4 new tests — snapshot rendering, granted-subset submission, Ctrl-C deny-all, and duplicate-request dedup.mcp_server_elicitation.rs,request_user_input/mod.rs: 1 new dedup test each.app.rs: 1 new test provingactive_session_events_via_app_server = truesuppresses the direct listener.chatwidget/tests.rs: existing tests updated for newInProcessAgentContextfields.chatwidget/agent.rs: ~1800 lines of test coverage for interactive request lifecycle, V1/V2 approval schemas, turn-scoped cleanup, auth token refresh.app-server-client:shared_thread_manager_observes_threads_started_in_process,shared_thread_manager_reuses_configured_auth_manager.core/auth.rs:nested_external_auth_override_inherits_lower_workspace_when_unset, override stacking tests.