fix: restore shared Codex spend in account menus - #3540
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 856af00263
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex review: needs real behavior proof before merge. Reviewed September 10, 2026, 4:13 PM ET / 20:13 UTC (Revision 4). ClawSweeper reviewWhat this changesRestores one shared local Codex cost section below stacked or compact account menus, preserving display preferences and adding regression coverage and a native proof harness. Merge readiness⛔ Blocked before merge - 2 items remain The fix remains useful, and the earlier code findings are addressed. No blocking correctness defect remains, but observable runtime proof is still incomplete. Priority: P2 Review scores
Verification
How this fits togetherCodexBar combines account usage with cost estimates from local Codex session logs. The menu now presents the ambient ledger outside individual account cards while retaining the existing cost-history submenu. flowchart TD
A[Local session logs] --> B[Scoped cost snapshot]
B --> C{Ambient Codex scope}
C --> D[Shared cost section]
E[Stacked or compact accounts] --> D
F[Cost display preference] --> D
D --> G[Inline summary and cost history]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep one explicitly labeled ambient cost section using existing presentation helpers, with account-scoped histories excluded and stored display preferences preserved. Do we have a high-confidence way to reproduce the issue? Yes, from source: with multiple visible Codex accounts and ambient cost data, main takes account-only renderers that omit the shared cost section. No runtime reproduction was executed during this review. Is this the best way to solve the issue? Yes. The common caller and existing cost views provide a narrow repair while preserving account isolation and all three display modes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against e9da07961168. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
856af00 to
9bceb85
Compare
9bceb85 to
8d7af20
Compare
|
@codex review The implementation changed substantially since the last review, so the previous findings no longer describe the code.
Worth scrutinising: whether the shared section belongs in |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Render ambient spend once after either account layout, preserving inline/submenu preferences and account-scoped isolation. Reuse existing cost views and keep Codex-specific trailing content out of the generic compact renderer. Co-authored-by: kays0x <48529172+kays0x@users.noreply.github.com>
8d7af20 to
9b44b2f
Compare
|
Merged as cebf999. The shared ambient ledger now appears once beneath stacked or compact Codex accounts, with inline-only, submenu-only and Both preferences respected. Account override cards and managed-profile history remain isolated. The proposal failed twelve layout/preference assertions before the maintainer fix. All 53 focused tests, the full 1,061-selection local suite, lint, independent P0–P2 review and all Linux/musl/macOS CI checks passed. Signed synthetic native proof covers both account layouts, all display modes and opening Cost history; inspected images are in the PR body. Provider docs and the Unreleased changelog are updated. Thanks @kays0x! |
Restores the shared ambient Codex ledger beneath multi-account cards. Both stacked and compact account layouts now reach one Codex-owned trailing section, labeled This Mac, without assigning the same spend to individual accounts. Only the existing
codex:ambientscope qualifies; managed-account and profile-home history remains isolated.The maintainer pass also fixes the proposal’s display-mode mismatch: inline-only uses the existing inline dashboard, submenu-only uses the existing compact Cost row, and Both shows each once. The generic compact renderer no longer needs a Codex-specific trailing callback. Account override-card isolation is unchanged.
Validation:
make check: zero violations across 2,173 Swift files. Independent P0–P2 review is clean.make test: 1,061 selections across 89 groups passed on the first attempt, with no retries or timeouts (787.6 seconds).9b44b2fe9a6ae85712fbbc7242b700997c11cbdc. Exact-head CI passed all Linux, musl and macOS checks. Merged ascebf9995f672d2c0c9ff3501b24721855b86b16c.Documentation and the Unreleased changelog explain the ambient scope and credit @kays0x.
Inspected synthetic native before/after proof
Legacy stacked rendering omits spend:
Stacked accounts with shared inline spend and Cost history:
Compact accounts with the Cost history submenu open:
Submenu-only mode keeps the compact Cost row: