Skip to content

Keep RPC method names out of the Codex limits help text - #13740

Closed
alanw707 wants to merge 1 commit into
omacom:quattrofrom
alanw707:codex-limits-readable-error
Closed

alanw707 wants to merge 1 commit into
omacom:quattrofrom
alanw707:codex-limits-readable-error

Conversation

@alanw707

Copy link
Copy Markdown

Follow-up to #13733.

Problem

When a Codex limits read fails, fetch_codex_rpc() writes str(exc) into authHelpText. rpc_request raises TimeoutError(method), so the Agents panel's help card shows the bare RPC method name. Before #13733 that was account/read; now it's account/rateLimits/read.

The panel shows authHelpText as guidance under "Codex limits unavailable", so a method name that looks like a permission reads as a sign-in or authorization problem. It sent me looking for a Codex sign-out/sign-in option, even though Codex was signed in and working the whole time.

Change

  • A timeout now says Codex didn't answer the usage request in time and that it will retry on the next refresh. Any other failure gets a generic "Couldn't read usage limits from Codex."
  • The unchanged usageStatusText ("Codex limits unavailable") stays as it was.
  • Added a scanner test in which account/rateLimits/read never answers. It asserts that the status is reported and that no method name appears in the help text. The new test fails on quattro and passes with this change.

Testing

  • test/shell.d/agent-usage-codex-scanner-test.sh, agent-usage-update-test.sh and agents-panel-test.sh pass.
  • Omarchy 4.0.4-1, codex-cli 0.156.1.

@sanjyay

sanjyay commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

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

Tested commit 6cb7a0a41683404b7e0df99f0eee79e4f8184eba in a clean Omarchy test environment.

Verification Summary

  • Base behavior on quattro: When rpc_request timed out reading limits from Codex, TimeoutError(method) raised the method name (such as "account/rateLimits/read"), which leaked directly into authHelpText. Because this field is rendered in the Agents panel under "Codex limits unavailable", it misleadingly resembled an authentication or permission error rather than a transient read timeout.
  • PR behavior: Differentiates TimeoutError to display clear user-facing guidance ("Codex didn't answer the usage request in time. It will retry on the next refresh.") and provides a generic fallback ("Couldn't read usage limits from Codex.") for other exceptions, preventing internal RPC endpoint paths from leaking to the UI.
  • Automated tests:
    • The new test case in test/shell.d/agent-usage-codex-scanner-test.sh cleanly reproduces the defect when run against base quattro ("authHelpText": "account/rateLimits/read", not ok - Codex collector keeps RPC method names out of the help text).
    • On the PR branch, all 24 assertions in agent-usage-codex-scanner-test.sh pass.
    • Related test suites test/shell.d/agent-usage-update-test.sh and test/shell.d/agents-panel-test.sh pass without issues.
    • Python compilation and bash syntax checks pass cleanly.
Test details
# Base quattro failure reproducing leaked RPC method name:
$ bash test/shell.d/agent-usage-codex-scanner-test.sh
...
{"schemaVersion":1,"id":"codex",...,"usageStatusText":"Codex limits unavailable","authHelpText":"account/rateLimits/read"}
not ok - Codex collector keeps RPC method names out of the help text

# PR branch execution:
$ bash test/shell.d/agent-usage-codex-scanner-test.sh
ok - Codex collector uses the supported approval policy
...
ok - Codex collector reads limits even when account/read never answers
ok - Codex collector keeps RPC method names out of the help text

$ bash test/shell.d/agent-usage-update-test.sh
ok - update reports a failing collector
...
ok - update with agent arguments only runs the named collectors

$ bash test/shell.d/agents-panel-test.sh
ok - agents panel launches the default agent
...
ok - agents right click no longer refreshes

$ python3 -m py_compile bin/omarchy-agent-usage-codex
$ bash -n test/shell.d/agent-usage-codex-scanner-test.sh

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

@jandrusk

jandrusk commented Oct 1, 2026

Copy link
Copy Markdown

Agree that stuffing str(exc) / TimeoutError(method) into authHelpText reads like a sign-in problem. Mapping timeout vs other failures to plain language is the right UX.

Two merge notes:

  1. This PR is currently dirty/conflicting with quattro — needs a rebase. Same files as Fix Codex limits timing out on buffered app-server replies #13703 / Handle interleaved Codex RPC notifications without timing out (#13773) #13816 / Describe Codex rate-limit RPC timeouts instead of the method name #13059 (bin/omarchy-agent-usage-codex + scanner test).
  2. Overlaps Describe Codex rate-limit RPC timeouts instead of the method name #13059 ("Describe Codex rate-limit RPC timeouts instead of the method name"). Prefer one of these UX PRs (or fold the help-text mapping into whichever race fix lands first) so we don't thrash the collector twice.

The race that makes limits unavailable in the first place is still #13703 / #13816 / #13773 — this PR only cleans the failure text.

@dhh

dhh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Thanks! The same fix is coming in through #14049 (from #8977, which replaces bare method names with the CLI's own error). Closing in favor of that.

@dhh dhh closed this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants