Skip to content

Streamline thread mutation handlers#19493

Merged
pakrym-oai merged 1 commit intomainfrom
pakrym/appserver-errors-thread-mutations
Apr 27, 2026
Merged

Streamline thread mutation handlers#19493
pakrym-oai merged 1 commit intomainfrom
pakrym/appserver-errors-thread-mutations

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

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

Why

Thread mutation handlers had many short error branches whose only job was to emit a JSON-RPC error and stop. This slice keeps those errors visible, but lets each handler build a result and return early from validation helpers instead of nesting the main path.

What Changed

  • Streamlined thread archive/unarchive, rename, memory, metadata, rollback, compact, background terminal, shell, and guardian handlers in codex-rs/app-server/src/codex_message_processor.rs.
  • Reused shared JSON-RPC error constructors in codex-rs/app-server/src/bespoke_event_handling.rs for rollback-related request failures.
  • Preserved direct send_error calls where they remain the simplest boundary for pending async event responses.

Verification

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

@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from da2717b to b8828ca Compare April 25, 2026 03:09
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-start branch from 9cfedc6 to 2a102a7 Compare April 25, 2026 03:09
@pakrym-oai pakrym-oai changed the title Streamline thread mutation app-server handlers Streamline thread mutation handlers Apr 25, 2026
@pakrym-oai pakrym-oai marked this pull request as ready for review April 25, 2026 04:00
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

self.outgoing.send_error(request_id, error).await;

P0 Badge Make thread_list_response return a proper Result

thread_list now delegates to thread_list_response, but this helper still calls send_error(request_id, ...) and uses bare return;. In this function, request_id is out of scope and the control flow no longer returns Result<ThreadListResponse, JSONRPCErrorError>. This introduces a build-breaking regression in the thread/list path.

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

@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from b8828ca to 558676d Compare April 25, 2026 04:10
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-start branch from 2a102a7 to 2d38a30 Compare April 25, 2026 04:10
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from 558676d to 8f1b0a0 Compare April 25, 2026 04:15
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-start branch 2 times, most recently from 2ecea71 to a6f9880 Compare April 26, 2026 23:08
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch 3 times, most recently from e89693c to c4b7cab Compare April 27, 2026 17:22
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-start branch from 37de240 to c641ef2 Compare April 27, 2026 17:22
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from c4b7cab to 6001c85 Compare April 27, 2026 18:45
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-start branch 2 times, most recently from 571c6c6 to 559a855 Compare April 27, 2026 20:38
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from 6001c85 to 016897c Compare April 27, 2026 20:38
Base automatically changed from pakrym/appserver-errors-thread-start to main April 27, 2026 20:56
@pakrym-oai pakrym-oai force-pushed the pakrym/appserver-errors-thread-mutations branch from 016897c to 869e8b6 Compare April 27, 2026 21:01
@pakrym-oai pakrym-oai merged commit 5c30d79 into main Apr 27, 2026
25 checks passed
@pakrym-oai pakrym-oai deleted the pakrym/appserver-errors-thread-mutations branch April 27, 2026 21:18
@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