Skip to content

Prevent clients racing with idle-timeout broker shutdown #526

Description

@HoneyTyagii

Summary

The new broker idle-timeout path can hang during shutdown, or leave a newly connected client attached to a broker whose underlying Codex app-server client has already been closed.

This is a follow-up to the idle-timeout work for #108 and is distinct from the explicit shutdown problems in #416.

Evidence

In plugins/codex/scripts/app-server-broker.mjs, the idle timer calls shutdown(server) and exits only after that promise settles.

shutdown() currently:

  1. Iterates the current sockets set and calls socket.end().
  2. Awaits appClient.close().
  3. Calls server.close().

The listening server therefore remains able to accept connections during the asynchronous appClient.close() window. A connection accepted in that interval is added to sockets, but it missed the earlier socket-closing loop.

When server.close() is finally called, Node waits for that newly accepted connection to close. If it remains open, shutdown() does not resolve and the process.exit(0) path does not run. Requests on that connection may also reach a broker whose app-server client has already been closed.

tests/broker-idle-timeout.test.mjs covers:

  • Timeout with no connected client
  • A client connected before timeout
  • Timeout-disabled behavior

It does not cover a connection racing with shutdown after the timer fires.

Expected behavior

Idle-timeout shutdown should transition the broker atomically into a non-accepting state before asynchronous cleanup begins. A client racing with timeout expiry should either be rejected or closed, or cancel shutdown before app-server teardown starts.

Acceptance criteria

  • Stop accepting new connections before awaiting appClient.close() or other asynchronous teardown.
  • Handle connections racing with timeout shutdown deterministically so they cannot keep the process alive indefinitely.
  • Ensure endpoint and PID-file cleanup still completes.
  • Add a regression test that connects after timeout shutdown starts and verifies bounded, clean broker exit.
  • Preserve current behavior for clients connected before timeout and when the timeout is disabled.
  • Cover Unix sockets and Windows named pipes where handling differs.

Affected files

  • plugins/codex/scripts/app-server-broker.mjs
  • tests/broker-idle-timeout.test.mjs

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions