Skip to content

Streamline account and command handlers#19491

Merged
pakrym-oai merged 1 commit intomainfrom
pakrym/appserver-errors-account-exec
Apr 27, 2026
Merged

Streamline account and command handlers#19491
pakrym-oai merged 1 commit intomainfrom
pakrym/appserver-errors-account-exec

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai commented Apr 25, 2026

Why

Account login/logout and command exec handlers were doing local error sends in the middle of each handler. That made these request flows branch heavily even though most of the logic is validate, perform the operation, and return the response.

What Changed

  • Converted ChatGPT/API-key login, login cancel, logout, rate-limit, and add-credit handlers in codex-rs/app-server/src/codex_message_processor.rs to compute Result values and send them once at the request boundary.
  • Applied the same shape to command exec start/write/resize/terminate handlers.
  • Kept side-effect notifications in the same places after successful request handling.

Verification

  • cargo check -p codex-app-server
  • cargo test -p codex-app-server --test all v2::account -- --test-threads=1
  • cargo test -p codex-app-server --test all v2::command_exec -- --test-threads=1

@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-plugins-apps-skills branch from 633d4a6 to 13c6797 Compare April 25, 2026 03:09
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch from a35af4e to 91f4e5d Compare April 25, 2026 03:09
@pakrym-oai pakrym-oai changed the title Streamline account and command app-server handlers Streamline account and command handlers Apr 25, 2026
@pakrym-oai pakrym-oai marked this pull request as ready for review April 25, 2026 04:00
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch from 91f4e5d to b4c0c48 Compare April 25, 2026 04:10
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-plugins-apps-skills branch from 13c6797 to 244a3ff Compare April 25, 2026 04:10
Copy link
Copy Markdown
Contributor

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

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: 91f4e5d250

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

.exec_one_off_command_inner(request_id.clone(), params)
.await
.map(|()| None::<serde_json::Value>);
self.send_optional_result(request_id, result).await;
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.

P0 Badge Replace undefined send_optional_result call

exec_one_off_command now calls self.send_optional_result(...), but this commit does not define that method on CodexMessageProcessor or import any trait that provides it. This makes codex_message_processor.rs fail to compile when type-checking this function, blocking the app-server build.

Useful? React with 👍 / 👎.

@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch 2 times, most recently from 35256b6 to 212b3b9 Compare April 26, 2026 23:08
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-plugins-apps-skills branch from 244a3ff to c6c2d92 Compare April 26, 2026 23:08
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch from 212b3b9 to abf4831 Compare April 26, 2026 23:15
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-plugins-apps-skills branch from c6c2d92 to a09b616 Compare April 26, 2026 23:15
Base automatically changed from pakrym/appserver-errors-plugins-apps-skills to main April 27, 2026 17:18
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch from abf4831 to 8419a48 Compare April 27, 2026 17:22
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-account-exec branch from 8419a48 to 4355894 Compare April 27, 2026 18:45
@pakrym-oai pakrym-oai enabled auto-merge (squash) April 27, 2026 18:56
@pakrym-oai pakrym-oai disabled auto-merge April 27, 2026 18:56
@pakrym-oai pakrym-oai merged commit e5709db into main Apr 27, 2026
25 checks passed
@pakrym-oai pakrym-oai deleted the pakrym/appserver-errors-account-exec branch April 27, 2026 19:03
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant