Handle interleaved Codex RPC notifications without timing out (#13773)

[szaidi-code]

Closes #13773

Summary

When Codex CLI sends notifications (such as remoteControl/status/changed or account/updated) in the same buffer stream following initialize, Python buffered text I/O reads the lines into memory. The subsequent select.select() on the underlying OS file descriptor reported not ready, leading rpc_request to miss replies already in memory and time out with "Codex limits unavailable".

Changes

• Replace select.select() on buffered proc.stdout with a dedicated queue-based thread reader and deadline timeout in bin/omarchy-agent-usage-codex.
• Add test coverage in test/shell.d/agent-usage-codex-scanner-test.sh asserting limits are read when notifications are interleaved.

[sanjyay]

Independent community review

This is an independent community review and unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Tested: Threaded RPC response pump, queue consumption timeouts, batched socket writes, and test suite execution in an isolated Omarchy test VM environment.

Findings & Feedback:
The thread pump approach (start_rpc_reader + queue.Queue) successfully avoids the buffered select.select() deadlock on proc.stdout. A few observations from testing:

1. Prior open work (Fix Codex limits timing out on buffered app-server replies #13703):
   PR Fix Codex limits timing out on buffered app-server replies #13703 addresses this same issue (Agents panel: Codex limits always "unavailable" — rpc_request misses replies already buffered by readline() #13773) using a non-threaded os.read(proc.stdout.fileno(), 65536) line splitter with an internal _rpc_pending buffer. It preserves the original rpc_request(proc, request_id, method, ...) signature without spawning background daemon threads. It may be worth comparing the two approaches with maintainers.

2. Test fixture note:
   In test/shell.d/agent-usage-codex-scanner-test.sh, the newly added test case with CODEX_INTERLEAVED_NOTIFICATIONS=1 passes even on unpatched origin/quattro. In that mock, the notifications follow the initialize reply, and the subsequent account/rateLimits/read request triggers a fresh write() from the mock server that wakes select.select().
   The actual deadlock occurs when a notification arrives in the same write ahead of a reply (e.g. printf '%s\n%s\n' '<notification>' '<reply>'), which drains the kernel pipe into Python's user buffer and strands the reply. When tested against such a batched write:

   • Base quattro: times out after 8s with "authHelpText": "account/rateLimits/read".
   • PR branch pr-13816 (2bb8e74): successfully consumes the queued reply and returns limits immediately (< 0.1s).
     Enhancing the mock in the test suite to emit the notification in the same write ahead of a reply would ensure the test fails on base and catches future regressions.

Testing details

• Tested PR commit: 2bb8e746eb864218ded11c8f96cf1ca7fd638a51
• Test comparison against batched notification + reply write:
  • Base origin/quattro:

    "usageStatusText": "Codex limits unavailable", "authHelpText": "account/rateLimits/read" (8s timeout)
    

  • PR branch (pr-13816):

    "limits": [{"label": "5h window", "percent": 0.05}], "tierLabel": "plus", "usageStatusText": ""
    

• Full test suite:
  • bash test/shell.d/agent-usage-codex-scanner-test.sh: 24/24 passed
  • git diff --check origin/quattro: clean

If you'd prefer not to receive these independent reviews on your PRs, reply to this comment, and I won't review your future PRs.

[aholbreich]

Triage note, tested 2026-10-01 against quattro (8b4eae66):

• This PR and Fix Codex limits timing out on buffered app-server replies #13703 fix the same race from Agents panel: Codex limits always "unavailable" — rpc_request misses replies already buffered by readline() #13773: a reply that arrives in the same write as a notification stays in Python's buffer, so select() never wakes up for it. Both collectors pass Fix Codex limits timing out on buffered app-server replies #13703's regression test.
• This PR's new test also passes on unpatched quattro, so it doesn't catch the bug (as @sanjyay noted above). Fix Codex limits timing out on buffered app-server replies #13703's test fails on quattro and passes with its fix, and @ibarani verified it independently with a separate harness.
• So I'd suggest merging Fix Codex limits timing out on buffered app-server replies #13703 and closing this one, along with Fix Codex usage timeouts on batched replies #12979 and Read codex app-server replies unbuffered in the usage collector #13102 (the same fix; both conflict with quattro now).

Separately, #13733 already covers #13458 (longer timeout) and #13464 (account/read optional). Issues #12880, #13266, #13425, #13880 and the timeout half of #13158 look fixed by it too. Worth a confirmation from the reporters on a build that includes #13733.

[jandrusk]

Triage (not posted): Prefer #13703 for #13773.

This PR and #13703 both target the Codex app-server race where a reply arrives in the same write as a notification. Community testing against quattro notes that this PR’s new interleaved-notification test also passes on unpatched quattro, so it does not catch the buffered-readline/select miss that #13703’s regression test covers. This branch is also CONFLICTING with quattro.

If maintainers want the queue/thread reader approach instead of #13703’s raw-fd + pending buffer, please rebase and add a test that fails on current quattro (notification + reply co-buffered before the next select). Otherwise closing this in favor of #13703 keeps one fix for the cluster (#13773, #13425, #13080, …).

[dhh]

Thanks! The same fix is coming in through #14049 (from #13703, which handles interleaved notifications without a reader thread). Closing in favor of that.