Skip to content

Shut down the in-process outbound router explicitly - #35523

Merged
copyberry[bot] merged 1 commit into
mainfrom
copyberry/codex-internal-to-codex-oss/a8f88d88a0950f50339b1c56909fedb781f178d4
Jul 26, 2026
Merged

Shut down the in-process outbound router explicitly#35523
copyberry[bot] merged 1 commit into
mainfrom
copyberry/codex-internal-to-codex-oss/a8f88d88a0950f50339b1c56909fedb781f178d4

Conversation

@copyberry

@copyberry copyberry Bot commented Jul 26, 2026

Copy link
Copy Markdown

Shut down the in-process outbound router explicitly

Why

Detached processor work can retain outgoing message senders, so waiting for
channel closure can keep the in-process app-server's outbound router alive
during shutdown.

What changed

Add an explicit shutdown signal for the outbound router and prioritize it over
pending outgoing messages. Send that signal after processor tasks are stopped
so runtime shutdown no longer depends on every sender being dropped.

Testing

Add a paused-time test that verifies the router exits and closes its channel
even when an outgoing sender is still retained.

## Why

Detached processor work can retain outgoing message senders, so waiting for
channel closure can keep the in-process app-server's outbound router alive
during shutdown.

## What changed

Add an explicit shutdown signal for the outbound router and prioritize it over
pending outgoing messages. Send that signal after processor tasks are stopped
so runtime shutdown no longer depends on every sender being dropped.

## Testing

Add a paused-time test that verifies the router exits and closes its channel
even when an outgoing sender is still retained.

GitOrigin-RevId: a8f88d88a0950f50339b1c56909fedb781f178d4
@copyberry
copyberry Bot force-pushed the copyberry/codex-internal-to-codex-oss/a8f88d88a0950f50339b1c56909fedb781f178d4 branch from 08e0a34 to 5a3808b Compare July 26, 2026 22:12
@copyberry
copyberry Bot merged commit 5a3808b into main Jul 26, 2026
14 of 32 checks passed
@copyberry
copyberry Bot deleted the copyberry/codex-internal-to-codex-oss/a8f88d88a0950f50339b1c56909fedb781f178d4 branch July 26, 2026 22:13

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

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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08e0a34693

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

drop(outgoing_tx);

let (shutdown_tx, shutdown_rx) = oneshot::channel();
let mut outbound_handle = tokio::spawn(run_outbound_router(

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.

P2 Badge Exercise the production shutdown path

When detached processor work retains an outgoing sender, this helper-level test never enters start_uninitialized or InProcessClientHandle::shutdown; it manually supplies the shutdown signal whose production wiring and ordering are the behavior under review. Removing or misplacing outbound_shutdown_tx.send(()) would therefore leave this test green while runtime shutdown again waits for its fallback timeout. Drive the scenario through the in-process runtime lifecycle, or an equivalent lifecycle fixture, so the regression is actually covered.

AGENTS.md reference: AGENTS.md:L252-L254

Useful? React with 👍 / 👎.

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 26, 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