Skip to content

Launch Codex app-server with an approval policy the CLI accepts - #7770

Closed
carinaesoft wants to merge 2 commits into
omacom:quattrofrom
carinaesoft:codex-app-server-approval-policy
Closed

carinaesoft wants to merge 2 commits into
omacom:quattrofrom
carinaesoft:codex-app-server-approval-policy

Conversation

@carinaesoft

Copy link
Copy Markdown

Fixes #7648.

Codex 0.149 retired the untrusted approval policy (openai/codex#39630), so the collector's launch never reaches app-server:

$ codex -s read-only -a untrusted app-server
error: invalid value 'untrusted' for '--ask-for-approval <APPROVAL_POLICY>'
  [possible values: on-request, never]

fetch_codex_rpc discards app-server's stderr and waits on the initialize reply, so the immediate exit surfaces as TimeoutError("initialize"). The panel then shows Codex limits unavailable over the bare method name — the symptom several people reported in the issue. Local token and session stats keep working, since they come off the session files.

Asking for on-request restores the limits. It is the closest surviving policy to the original intent: nothing is auto-approved, -s read-only still bounds the sandbox, and the app-server is only ever read from (account/read, account/rateLimits/read).

Test

test/shell.d/agent-usage-codex-limits-test.sh stubs codex the way the real CLI behaves: it validates --ask-for-approval before speaking the protocol, and rejects a retired value. The test fails on quattro with the reported authHelpText: "initialize" and passes with the fix.

ok - Codex collector reads the window app-server reports
ok - Codex collector names the plan app-server reports
ok - Codex collector launches app-server with an approval policy the CLI accepts

Verified against a live Codex 0.149.0 sign-in too: the collector returns tierLabel: "team", an empty usageStatusText, and the weekly window.

./test/all is otherwise unchanged — the 4 failures on this machine (bar-icon-geometry, config, snapper, unowned-system-paths) reproduce on a clean quattro checkout.

Not in this PR

The issue also notes that an early app-server exit should not be reported as the pending RPC method name. That is a separate change to the error handling and I left it out to keep this atomic — happy to follow up if you want it.

Codex 0.149 retired the "untrusted" approval policy, so the collector's
launch is rejected before app-server starts and the limits RPC times out.
The panel then reports "Codex limits unavailable" over the pending method
name, "initialize".

Ask for "on-request" instead; the read-only sandbox is unchanged, and the
app-server is only read from.

Fixes omacom#7648

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 22, 2026 12:23

Copilot AI 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.

Pull request overview

Updates the Codex usage collector for Codex 0.149 compatibility.

Changes:

  • Replaces retired untrusted policy with on-request.
  • Adds regression coverage for app-server limits retrieval.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
bin/omarchy-agent-usage-codex Uses a supported Codex approval policy.
test/shell.d/agent-usage-codex-limits-test.sh Verifies app-server startup and limit parsing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

docs/testing.md gives every shell test a `set -euo pipefail` before it
sources base-test.sh, and 154 of the 187 files under test/shell.d follow
it. Without it a failed `mktemp -d` leaves TEST_HOME empty and the
fixture is built at the filesystem root instead of aborting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed by Claude Opus 5 with an independent second opinion from Codex at xhigh reasoning. The diagnosis and the fix both hold up. Everything below was executed on a disposable VM built from the Omarchy ISO with codex-cli 0.149.0 installed, never on the machine doing the review.

The policy value. codex --help on 0.149.0 gives --ask-for-approval exactly two values, on-request and never; codex -s read-only -a untrusted app-server exits 2 before the server starts. Running the collector unchanged from this branch against that real binary reproduces the fix, and running it with untrusted restored reproduces the bug exactly as reported — "usageStatusText":"Codex limits unavailable","authHelpText":"initialize".

on-request versus never. Both are accepted and both work here, and the collector cannot hang on either: it never starts a thread or a turn, and account/read and account/rateLimits/read are dispatched by the app-server's account processor without touching the approval machinery, so no approval request can ever be raised for these three RPCs. Codex traced that through the 0.149 app-server sources and reached the same place independently. There are five other open pull requests fixing this same line and they split between the two values, so which one lands is the maintainer's call rather than a defect in this one.

The wire format. codex app-server generate-json-schema on 0.149.0 defines GetAccountRateLimitsResponse with rateLimits required and documented as "Backward-compatible single-bucket view; mirrors the historical payload", alongside an optional rateLimitsByLimitId. The field was not renamed, so reading result.rateLimits at bin/omarchy-agent-usage-codex:551 is still correct and this change is a complete fix for the reported symptom.

Tests. test/shell.d/agent-usage-codex-limits-test.sh and test/shell.d/agent-usage-codex-scanner-test.sh both pass on the worker (3 and 20 assertions), and ./test/cli passes with 116 assertions. Mutating the policy back to untrusted fails the new test on its first assertion while leaving the pre-existing scanner test green — which is the point of the new file, since that older stub answers the protocol whatever policy it is handed and so never saw this regression. The stub here matches the real CLI's accepted set, verified against the installed 0.149.0.

Pushed as 85707677: set -euo pipefail before source base-test.sh. docs/testing.md prescribes it for every shell test and 154 of the 187 files under test/shell.d have it; without it a failed mktemp -d leaves TEST_HOME empty and the fixture gets built at the filesystem root rather than aborting. Codex found this one; nothing else in the diff drew a finding from either reviewer. The tests above were re-run on the worker after that commit.

Not fixed, and not yours to fix here. rpc_request at bin/omarchy-agent-usage-codex:493 returns any message whose id matches without looking for a JSON-RPC error, so a signed-out machine — where account/rateLimits/read answers {"error": {"code": -32600, "message": "codex account authentication required to read rate limits"}} — falls through as an empty success and the panel shows "Subscription" with no limits and no sign-in hint, because the status card at shell/plugins/agents/Panel.qml:498 only renders when usageStatusText is non-empty. That is pre-existing and independent of the approval policy, and it is the same area you deliberately left out of scope, so it belongs in its own change.

This is now waiting on the maintainer, to pick one of the six pull requests against this line.

@upnorthmedia

Copy link
Copy Markdown

confirmed on my end. codex 0.149.0, aarch64 asahi. limits came back right after the change,
plan and weekly window both populating.

separate from the fix itself, but it's the reason this one was hard to see from outside.

when the flag gets rejected the cli says exactly what's wrong:

error: invalid value 'untrusted' for '--ask-for-approval <APPROVAL_POLICY>'
  [possible values: on-request, never]

that goes to DEVNULL, so all the collector sees is the process exiting, stdout closing, and
initialize timing out at 8s. the except path does authHelpText = str(exc), and rpc_request
raises TimeoutError(method), so the field ends up reading "initialize". in the panel that
reads as an auth problem, which sends you looking in the wrong place.

capturing stderr and preferring it would let the next cli flag change explain itself:

      stderr=subprocess.PIPE,
  except Exception as exc:
    result["usageStatusText"] = "Codex limits unavailable"
    detail = ""
    if proc.poll() is not None:
      try:
        detail = (proc.stderr.read() or "").strip().splitlines()[0]
      except Exception:
        pass
    result["authHelpText"] = detail or AUTH_HELP

the poll() guard keeps it from reading a live process, and falling back to AUTH_HELP keeps
an rpc method name out of a user facing string.

tested against 0.149.0 three ways. bad flag puts the cli error into authHelpText instead of
"initialize". good flag still returns limits and plan exactly like it does now. and a process
that starts but never answers falls back to AUTH_HELP at the timeout with no hang, which is
the case the poll() guard is there for.

happy to put this up as its own pr rather than have it ride along here, since AGENTS.md
asks for atomic commits.

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed after @upnorthmedia's confirmation. The branch is unchanged at 85707677 and still correct, so there is nothing new against the diff itself — what follows is an answer to that comment and to the merge-order question the cluster has created. Reviewed by Claude Opus 5 with an independent second opinion from Codex at xhigh reasoning; everything executed below ran on a disposable VM with codex-cli 0.149.0, never on the machine doing the review.

The mechanism is right, the timing is not. initialize never reaches its 8s deadline. The rejected argv makes codex exit before it speaks, so the child's stdout is already closed by the time the collector looks: select at line 483 reports it readable, readline at 486 returns EOF, if not line: break at 487-488 leaves the loop, and line 495 raises TimeoutError("initialize") at once. Measured on the worker at 42-44 ms, not 8 s. The conclusion is unchanged — authHelpText really does end up reading initialize — but anyone designing the diagnostic fix should know the timeout is not the path it takes.

On the stderr proposal. It is worth doing and it does not belong in this PR, so yes to a separate one. Checked against the source and against the real binary first, since it is easier to review now than after it is written:

  • The proc.poll() guard is not sufficient. read() on a pipe returns at EOF, and EOF needs every writer to have closed, not just the direct child — a grandchild that inherited the stderr fd keeps it open. read() would then block inside the except, before the finally at 562-570 can terminate anything, so the collector hangs instead of failing. Codex raised this and a probe on the worker confirms the state is reachable: poll() returned 7 for an exited child while read() had still not returned two seconds later. Whether codex app-server ever leaves such a grandchild is the thing to check; the guard is not a guarantee on its own.
  • An undrained PIPE can backpressure the child once 64 KiB accumulates on it. A healthy 0.149.0 wrote 0 bytes to stderr through startup here, so this is a caution to verify rather than a known fault.
  • (proc.stderr.read() or "").strip().splitlines()[0] behaves as you describe: empty stderr raises IndexError, the inner except swallows it, detail stays empty and AUTH_HELP wins.
  • One design note. On the live-process path, where poll() is None, the new code replaces today's str(exc) with AUTH_HELP — so a genuine timeout against a running app-server would say "Run codex login" instead of naming what timed out, which is the same class of misleading string the change exists to remove. Worth handling as its own case rather than falling through.
  • Codex claimed the CLI prints a warning line ahead of the error, which would make the first line the wrong line to show. That did not reproduce: on a clean Omarchy worker, codex -s read-only -a untrusted app-server writes exactly 152 bytes over 4 lines and line 1 is the error. The heuristic holds.

Why this PR is still worth taking, given #7649 is byte-identical on the production line. Both resolve bin/omarchy-agent-usage-codex to the same blob, they merge without conflict, and both test files pass together on the merged tree — so the two are complementary rather than competing, and the second production hunk is a no-op by the time it lands. The distinct property is in the test. This stub validates --ask-for-approval the way the real CLI does and asserts the user-visible outcome, rather than pinning the exact argv, so it catches the class of bug: mutating the collector back to -a untrusted fails it on the first assertion with "usageStatusText":"Codex limits unavailable","authHelpText":"initialize", while mutating to -a never leaves it green. That means it cannot go stale if the maintainer prefers never, and it would catch the next retired flag value the same way this one was caught. #7649's exact-argv assertion is the tighter check in the other direction — it also notices a dropped -s read-only. Taking both gets both properties, which is the case for merging this rather than closing it as a duplicate.

Tests, re-run on the worker at this head. test/shell.d/agent-usage-codex-limits-test.sh — 3 passed. test/shell.d/agent-usage-codex-scanner-test.sh — 21 passed. ./test/cli — 116 passed, exit 0. Under the untrusted mutation the new file fails while the scanner file stays green, which is the reason the new file earns its place.

Waiting on the maintainer, on a choice rather than a fix. Ten open PRs now change this one line: #7649 and #7770 on on-request, and #7751, #7825, #7829, #7864, #7892, #7908, #7911, #7924 on never. Both values are accepted by 0.149 and behaviourally identical on this path, since the collector never starts a turn. #7997 and #8032 are two further reports of the same breakage. Nothing pushed to this branch in this pass.

@omarchybot

Copy link
Copy Markdown
Collaborator

#7649 is merged (4cd8a08) and makes this same fix: the collector now launches app-server with -a on-request instead of the retired untrusted, with the launch arguments asserted in the collector test.

Same policy choice you argued for, and for the same reason. Closing as superseded — thanks for the write-up, including the note about the early-exit error being reported as the pending RPC method name; that one is still worth a follow-up if you want to send it.

@omarchybot omarchybot closed this Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex 0.149 breaks Agents panel limit display

4 participants