feat(tui): start wiring app-server backend behind --app-server

[fcoury]

Problem

The TUI talks directly to codex-core, duplicating business logic that the app-server already provides to other surfaces like the VSCode extension. This means behavior can diverge between surfaces, and any fix or enhancement must be replicated in both the TUI and the app-server independently. The goal is to migrate the TUI onto the app-server so all clients share a single implementation. Skills loading (ListSkills) is the first operation to move — it's self-contained, has clear request/response semantics, and exercises the full conversion pipeline — making it a good proving ground for the pattern the remaining operations will follow.

Mental model

There are now two skill-loading channels, selected by the --app-server CLI flag:

1. Legacy (default): ChatWidget sends Op::ListSkills over the codex-core op channel. Core handles the request end-to-end and fires EventMsg::ListSkillsResponse back.
2. App-server: ChatWidget wraps Op::ListSkills inside AppEvent::CodexOp(…) and sends it up to the App event loop. App::run intercepts it, expands empty cwds against the active chat session cwd, kicks off a detached typed skills/list RPC against the embedded app-server, converts the response from app-server protocol types into core protocol types, and pushes the result into the same set_skills_from_response / refresh_plugin_mentions path the legacy flow uses.

The decision point is ChatWidget::request_skills_list, which inspects use_app_server and routes to the appropriate channel. The App event loop's select! arm pattern-matches on AppEvent::CodexOp(Op::ListSkills { .. }) only when use_app_server is true; all other events fall through to the existing handle_event handler.

Non-goals

• Routing any operation other than ListSkills through the app-server. This is the first gated operation; others will follow the same pattern.
• Changing the app-server protocol types to match core types (or vice versa). The conversion functions are intentionally mechanical 1:1 mappings.
• Removing the legacy codex-core skills path. Both paths must work.

Tradeoffs

• ~130 lines of into_core_* boilerplate (8 functions): These could be derived with a macro or eliminated by unifying the protocol types, but field-by-field mapping keeps the two protocol crates decoupled and makes divergence between them compile-time visible.
• Hardcoded RequestId::Integer(1): The detached skills/list path still uses a static request ID. That is acceptable for the current narrowly scoped migration, but it is an explicit limitation and should become a counter before this pattern is reused more broadly.
• per_cwd_extra_user_roots: None: The TUI does not yet support per-CWD extra roots. This is deferred, not forgotten.

Architecture

CLI --app-server flag
  → lib.rs: extracted into `app_server` bool, passed to App::run
    → App::run: threaded into ChatWidgetInit.use_app_server
      → ChatWidget: stored as field, exposed via use_app_server()

ChatWidget::request_skills_list(cwds, force_reload)
  ├─ use_app_server=false → submit_op(Op::ListSkills) → codex-core op channel
  └─ use_app_server=true  → app_event_tx.send(AppEvent::CodexOp(Op::ListSkills))
                                ↓
                           App event loop select! arm
                                ↓
                           effective_skills_list_cwds
                                ↓
                           request_app_server_skills_list (tokio::spawn)
                                ↓
                           app_server.requester().request_typed(ClientRequest::SkillsList)
                                ↓
                           into_core_skills_list_response_event (+ into_core_* children)
                                ↓
                           app_event_tx.send(AppEvent::SkillsListLoaded { result })
                                ↓
                           App event loop handles SkillsListLoaded
                                ↓
                           chat_widget.set_skills_from_response / refresh_plugin_mentions

App-server adapter (app_server_adapter.rs)

A transitional adapter module on App that drains the app-server event stream during the hybrid migration period. It handles Lagged, ServerNotification, and LegacyNotification events, and rejects any ServerRequest with a JSON-RPC -32000 error since the TUI does not yet handle server-initiated requests. This module should shrink and disappear as more flows move onto the app-server surface.

Files changed outside tui/

The only non-TUI code touched by this branch is:

• app-server-client/src/lib.rs — Extended InProcessAppServerClient with typed request/response plumbing and an InProcessAppServerRequester handle for making RPC calls from spawned tasks.

Observability

• skills/list RPC failures surface as color_eyre errors that propagate up and terminate the app — no silent swallowing.
• Skill-load warnings (invalid SKILL.md files) are emitted via emit_skill_load_warnings on the app-server path identically to the legacy path.

Tests

• list_skills_without_app_server_uses_core_op_channel — verifies SkillsUpdateAvailable routes through the op channel when use_app_server=false, and nothing appears on the app-event channel.
• list_skills_with_app_server_uses_app_event_channel — verifies the inverse: op channel stays empty, app-event channel receives the wrapped op.
• effective_skills_list_cwds_uses_active_chat_widget_cwd_when_request_empty — verifies the app-server path uses the active session cwd rather than the original launch cwd when the request does not specify cwds.
• effective_skills_list_cwds_preserves_explicit_cwds — verifies explicitly provided cwd values are forwarded unchanged.
• parses_app_server_flag — confirms clap parses --app-server.
• Existing tests updated to pass use_app_server: false to ChatWidgetInit constructors, confirming no behavioral change in the default path.

[Copilot]

Pull request overview

This PR begins integrating the embedded app-server into the TUI by introducing a new runtime event layer (RuntimeEvent / AppServerEvent) so the outer App loop can own app-server work (starting with skills listing) and react to app-server notifications, while updating TUI tests/helpers to unwrap the new event envelope.

Changes:

• Add RuntimeEvent + AppServerEvent, and update AppEventSender to send RuntimeEvent.
• Route Op::ListSkills via an app-server request handled in the App runtime loop.
• Add --app-server CLI flag and update tests to receive RuntimeEvent::App(...).

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
codex-rs/tui/src/status_indicator_widget.rs	Update tests to use RuntimeEvent channels.
codex-rs/tui/src/public_widgets/composer_input.rs	Switch internal channel types to RuntimeEvent.
codex-rs/tui/src/lib.rs	Thread new app_server CLI flag through to App::run.
codex-rs/tui/src/cli.rs	Add --app-server boolean flag.
codex-rs/tui/src/chatwidget/tests.rs	Introduce a RuntimeEvent-unwrapping receiver helper for tests and update initialization.
codex-rs/tui/src/chatwidget.rs	Add app-server event usage and reroute skills listing into runtime lane; expose use_app_server() and widen refresh_plugin_mentions visibility.
codex-rs/tui/src/bottom_pane/status_line_setup.rs	Update tests to use RuntimeEvent channels.
codex-rs/tui/src/bottom_pane/skills_toggle_view.rs	Update tests to use RuntimeEvent channels.
codex-rs/tui/src/bottom_pane/request_user_input/mod.rs	Update tests to unwrap RuntimeEvent::App(...) via a local helper.
codex-rs/tui/src/bottom_pane/mod.rs	Update tests to use RuntimeEvent channels and unwrap RuntimeEvent::App(...).
codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs	Update tests to unwrap RuntimeEvent::App(...) via a local helper.
codex-rs/tui/src/bottom_pane/list_selection_view.rs	Update tests to use RuntimeEvent channels and unwrap RuntimeEvent::App(...).
codex-rs/tui/src/bottom_pane/feedback_view.rs	Update tests to use RuntimeEvent channels.
codex-rs/tui/src/bottom_pane/chat_composer_history.rs	Update tests to use RuntimeEvent channels and unwrap RuntimeEvent::App(...).
codex-rs/tui/src/bottom_pane/chat_composer.rs	Update tests to use RuntimeEvent channels and adjust pattern matches accordingly.
codex-rs/tui/src/bottom_pane/approval_overlay.rs	Update tests to use RuntimeEvent channels and unwrap RuntimeEvent::App(...).
codex-rs/tui/src/bottom_pane/app_link_view.rs	Update tests to use RuntimeEvent channels and unwrap RuntimeEvent::App(...).
codex-rs/tui/src/app_event_sender.rs	Change sender to emit RuntimeEvent and add send_app_server.
codex-rs/tui/src/app_event.rs	Introduce RuntimeEvent and AppServerEvent enums.
codex-rs/tui/src/app.rs	Handle RuntimeEvent in main loop; issue app-server skills/list; react to app/list/updated notifications.

---

💡 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.

[fcoury]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8dbe649a4

ℹ️ 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".

[fcoury]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc27416c00

ℹ️ 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".

[chatgpt-codex-connector]

Drop stale skills responses for the same cwd

skills/list calls are launched on detached tasks, so multiple requests for the same cwd can complete out of order; however this handler only rejects responses when the cwd changed, and otherwise applies whatever arrives last. In --app-server mode that allows an older response to overwrite a newer one (for example after rapid refreshes or enable/disable flows), leaving the UI with stale skill metadata until another reload. Consider tracking request recency (e.g., request_id/counter) and ignoring older completions even when cwd matches.

Useful? React with 👍 / 👎.

[etraut-openai]

I started to review. This was a good experiment because the ListSkills doesn't involve any events, so it's relatively straightforward. However, this highlights how challenging it's going to be to retain both legacy and app server code paths — even in the short term. It makes me question whether a feature flag is the right approach here.

I'm going to do an experiment and create a separate parallel PR that explores the approach where we don't make it conditional and just cut over to the new app server path so we can compare.

[etraut-openai]

It looks like it takes a lot of code to convert from app server to core data structures in this case. We'll eventually want the TUI to consume app server data structures directly, so maybe it's better to convert in the other direction? Or do you think we should do that as a separate (later) step?

[fcoury]

I have another branch where I do that. We can discuss the implementation. In this particular PR I wanted to do as little as possible to get this one action migrated.

[fcoury]

I started to review. This was a good experiment because the ListSkills doesn't involve any events, so it's relatively straightforward. However, this highlights how challenging it's going to be to retain both legacy and app server code paths — even in the short term. It makes me question whether a feature flag is the right approach here.

I'm going to do an experiment and create a separate parallel PR that explores the approach where we don't make it conditional and just cut over to the new app server path so we can compare.

@etraut-openai I share your concern. I think the clean cut is the right approach, but even that is not without its problems. If you check the last version of my PR, there are certain features that just can't be migrated (or not unless we implement them on AppServer).

The one that always comes to mind is the RunUserShellCommand. It runs the command and injects the result on the current context.

Another case is the shutdown. If you execute your branch today and hit Ctrl+C you'll notice that it takes quite some time to actually shutdown.

This is because we are running the local shutdown, which is fast but also shutdown the embedded app-server runtime. And this process drains the background tasks, waiting up to 10 minutes.

I think we need to explore moving all the remaining commands, even RunUserShellCommand and improving the shutdown as well.

This is a list of items we had migrated vs decided not to migrate, mostly to not change app-server on PR #14018.

Actions kept local in TUI

Handled locally (no app-server RPC needed):

• AddToHistory, GetHistoryEntryRequest, ListCustomPrompts

Forwarded directly to CodexThread (bypassing app-server, functional):

• ReloadUserConfig, OverrideTurnContext, DropMemories, UpdateMemories, RunUserShellCommand, ListMcpTools

Deferred (emit local warning, not functional):

• Undo — emits "temporarily unavailable in in-process local-only mode"

Shutdown is intentionally not a dedicated RPC — it maps to thread/unsubscribe plus local completion handling.

Notifications staying on the legacy event bridge

These notifications are not on the typed app-server path:

• warning, backgroundEvent, turn/streamError
• thread/undo/started, thread/undo/completed, thread/shutdown/completed
• mcpServer/startup/updated, mcpServer/startup/completed
• skills/updated

Three 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 → EventMsg bridge, including realtime event consumption (realtime requests were migrated, but event consumption was not).

Other non-migrated pieces

• account/chatgptAuthTokens/refresh remains a local server-request adapter rather than a deeper app-server-owned flow.