[codex] Refactor app-server dispatch result flow

[pakrym-oai]

Why

App-server request handling had response sending spread across many individual handlers, which made it harder to see which requests return payloads, which methods send their own delayed response, and which branches emit notifications after a response.

What changed

• Centralized normal ClientResponsePayload sending in the dispatch path.
• Kept explicit-response methods explicit where they need custom ordering or delayed delivery.
• Removed forward-only handler wrappers and immediate async { ... }.await bodies where they were not needed.
• Moved branch-specific post-response notifications into the branches that own the response ordering.
• Replaced unreachable delegated request-family error arms with explicit unreachable! cases.

Verification

• cargo check -p codex-app-server
• cargo test -p codex-app-server thread_goal
• just fix -p codex-app-server

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7599fb44d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Keep model list handling off the main request loop

The ModelList arm now awaits list_models inline, which can block the app-server dispatcher while model metadata is fetched. I checked app-server/src/lib.rs and incoming requests are handled sequentially by awaiting processor.process_request(...), so this change can stall unrelated requests/notifications until supported_models(...).await completes (it uses RefreshStrategy::OnlineIfUncached, which may perform network I/O on cache miss). Previously this path was spawned, preserving loop responsiveness.

Useful? React with 👍 / 👎.

[etraut-openai]

Nice simplification!

One small concern is that we're now moving to a mixed approach for handlers: complicated handlers return None and do their own completion handling whereas the simpler ones rely on the common handling. A None response can mean a number of different things: response already sent, response deferred to a task, a delegated handler owns the response, etc. That could mean that future code changes (especially new handlers) could return None inappropriately, and it would be tough to catch. One idea is to define a new enum that covers these different cases. This would allow for better tests and help codex (and human reviewers) reason about each return path. I wouldn't hold the PR for this, but it's something to consider as a potential follow-up.

This change is big enough that I didn't review every line, but I did skim through it. I also interrogated Codex to look for various problems. And I built and ran it locally to do some basic smoke tests on features that are affected by the modified code paths. I didn't find any issues.

Codex left a code review comment in the PR thread about model/list. I don't think this is a bug, but it is a subtle behavioral change. The collaboration-mode/list api appears to be affected in the same way.