Repository navigation
Launch Codex app-server with an approval policy the CLI accepts - #7770
carinaesoft wants to merge 2 commits into
Conversation
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>
There was a problem hiding this comment.
Pull request overview
Updates the Codex usage collector for Codex 0.149 compatibility.
Changes:
- Replaces retired
untrustedpolicy withon-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>
|
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.
The wire format. Tests. Pushed as Not fixed, and not yours to fix here. This is now waiting on the maintainer, to pick one of the six pull requests against this line. |
|
confirmed on my end. codex 0.149.0, aarch64 asahi. limits came back right after the change, 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: that goes to DEVNULL, so all the collector sees is the process exiting, stdout closing, and 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_HELPthe poll() guard keeps it from reading a live process, and falling back to AUTH_HELP keeps tested against 0.149.0 three ways. bad flag puts the cli error into authHelpText instead of happy to put this up as its own pr rather than have it ride along here, since AGENTS.md |
|
Re-reviewed after @upnorthmedia's confirmation. The branch is unchanged at The mechanism is right, the timing is not. 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:
Why this PR is still worth taking, given #7649 is byte-identical on the production line. Both resolve Tests, re-run on the worker at this head. Waiting on the maintainer, on a choice rather than a fix. Ten open PRs now change this one line: #7649 and #7770 on |
|
#7649 is merged (4cd8a08) and makes this same fix: the collector now launches app-server with 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. |
Fixes #7648.
Codex 0.149 retired the
untrustedapproval policy (openai/codex#39630), so the collector's launch never reaches app-server:fetch_codex_rpcdiscards app-server's stderr and waits on theinitializereply, so the immediate exit surfaces asTimeoutError("initialize"). The panel then showsCodex limits unavailableover 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-requestrestores the limits. It is the closest surviving policy to the original intent: nothing is auto-approved,-s read-onlystill 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.shstubscodexthe way the real CLI behaves: it validates--ask-for-approvalbefore speaking the protocol, and rejects a retired value. The test fails onquattrowith the reportedauthHelpText: "initialize"and passes with the fix.Verified against a live Codex 0.149.0 sign-in too: the collector returns
tierLabel: "team", an emptyusageStatusText, and the weekly window../test/allis otherwise unchanged — the 4 failures on this machine (bar-icon-geometry,config,snapper,unowned-system-paths) reproduce on a cleanquattrocheckout.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.