Repository navigation
fix(cli): omit stale supplemental OpenAI dashboard caches - #4882
Chipagosfinest wants to merge 3 commits into
Conversation
|
@clawsweeper re-review |
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
🦞👀 Re-review progress:
|
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. |
|
Codex review: blocked before merge. What this changesThis PR omits supplemental cached OpenAI dashboards older than 15 minutes or more than 60 seconds ahead of attachment time. Example: OAuth returns fresh 10% session and 20% weekly usage alongside an ownership-compatible, 65-day-old dashboard cache.
Review scores
ProductKind: Bug fix · Worth it: Needs a maintainer decision · Fix scope: Complete Merge readiness⛔ Blocked before merge - 2 items remain This PR addresses a source-confirmed stale-cache bug with a focused patch and sufficient CLI proof; the proposed freshness bounds still need an owner decision. Priority: P2 Decision needed
Before merge
FindingsNone. Agent review detailsHow this fits togetherThe CLI supplements successful Codex usage results with a disk-cached dashboard after shared account-ownership validation, then serializes the optional supplement into usage JSON. flowchart LR
A[Usage command] --> B[Codex provider fetch]
B --> C[Fresh usage result]
D[Dashboard disk cache] --> E[Ownership validation]
C --> E
E --> F[Cache age check]
F --> G[Usage JSON]
Technical reviewBest possible solution: Retain the narrow attachment-time check and cache preservation, with freshness bounds explicitly accepted by the feature owner. Do we have a high-confidence way to reproduce the issue? Main's attachment path has no age guard, and the supplied native CLI comparison demonstrates a 65-day-old dashboard attached beside current OAuth usage; no target code was executed during this read-only review. Is this the best way to solve the issue? The patch fixes the attachment boundary directly without deleting historical data, adding configuration, or changing shared ownership decisions; the numerical freshness policy remains an owner choice. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 897bff31731b. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (3 earlier review cycles)
Reviewed November 1, 2026, 5:11 PM ET / 22:11 UTC (Revision 4). |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
Native Mac CLI proof against freshly fetched main
All commands exit 0 and retain fresh 10% / 20% quotas. Cache, auth, and config remain byte-identical after every command. Actual CLI trace · 12 focused tests passed, including six age cases · provenance. No real account data or product-code changes. |
Caches stamped up to 60s ahead of now (small clock corrections) still attach; the 15-minute limit is a named constant; the unrelated import reorder is reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Codex usage JSON can attach an ownership-compatible dashboard cache beside fresh OAuth usage even when the cached balances and limits are months old. A QA capture paired current OAuth usage with a 65-day-old dashboard whose weekly reset was already in the past.
Changes:
openaiDashboardonly when itsupdatedAtis within the preceding 15 minutes, allowing timestamps up to 60 seconds ahead for clock skew. Expired caches and those more than 60 seconds ahead are omitted.Validation uses synthetic auth homes and per-test dashboard caches; no real provider probes or Keychain access.
swift:6.3-noble(Linux ARM64,libsqlite3-dev+ Python): all 12CLIOpenAIDashboardCacheTestspass, including age cases at 0, 60, 900, 901, 5,616,000 (65 days), and -60 seconds. The actual cache test suite and its credential fixture were copied intoTestsLinuxinside the isolated container.make checkpassed with the installed Python 3.13 onPATHandTOOLCHAIN_DIR=/Library/Developer/CommandLineTools; SwiftFormat and strict SwiftLint passed.swift build --product CodexBarCLIpassed natively on macOS. App-dependent macOS tests could not build locally because this CLT-only Mac lacks Xcode'sactool; macOS CI remains the gate for those suites.source ~/.config/council/openrouter.env && council review --base upstream/main: 0 retained findings on the submitted commit. Independent read-only review also found no concrete defects.Duplicate search refreshed before implementation: no exact match for the optional CLI cache attachment. #4718 / #4707 address serve snapshot refresh behavior.
Earlier shipped CLI behavior and stable-cache upgrade proof (before clock-skew allowance)
Ran the installed stable 0.83.0 CLI and the freshly built CLI at
c7062634745db4be16f82e33baff5821361b144bthrough their realusage --provider codex --source oauth --jsonentry point, each in a fresh process. A loopback HTTP fixture supplied current OAuth primary/weekly usage (10% / 20%) through the existingchatgpt_base_urlconfig option. This exercises OAuth HTTP fetch → provider mapping → disk-cache ownership and age policy → usage JSON serialization. No attachment helper or prebuilt command-output injection is used.The home, Codex auth/config, and Application Support cache are synthetic; the process gets a clean environment with Keychain access disabled. No real account or remote provider request is used. These are fixture-driven integration results, not live OpenAI service proof.
The same cache bytes are first read successfully by the installed stable binary, then passed unchanged to the patched binary. SHA-256 comparisons verify that the dashboard cache, CLI settings, Codex config and synthetic auth file remain unchanged after every invocation. This covers upgrade compatibility with a stable-readable existing cache and settings without touching the user's real cache/settings.
Command for both binaries (paths point to the synthetic fixture):
Terminal output projected to non-sensitive behavior fields:
{"binary": "stable-0.83.0", "cache": "recent", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:41Z", "dashboardAttached": true, "cacheUnchanged": true, "settingsUnchanged": true, "dashboardUpdatedAt": "2026-11-01T05:59:40Z"} {"binary": "patched-c706263", "cache": "recent", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:42Z", "dashboardAttached": true, "cacheUnchanged": true, "settingsUnchanged": true, "dashboardUpdatedAt": "2026-11-01T05:59:40Z"} {"binary": "stable-0.83.0", "cache": "expired-65-days", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:43Z", "dashboardAttached": true, "cacheUnchanged": true, "settingsUnchanged": true, "dashboardUpdatedAt": "2026-08-28T06:00:40Z"} {"binary": "patched-c706263", "cache": "expired-65-days", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:44Z", "dashboardAttached": false, "cacheUnchanged": true, "settingsUnchanged": true} {"binary": "stable-0.83.0", "cache": "future", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:45Z", "dashboardAttached": true, "cacheUnchanged": true, "settingsUnchanged": true, "dashboardUpdatedAt": "2026-11-01T07:00:40Z"} {"binary": "patched-c706263", "cache": "future", "exit": 0, "source": "oauth", "usageUpdatedAt": "2026-11-01T06:00:47Z", "dashboardAttached": false, "cacheUnchanged": true, "settingsUnchanged": true}All six commands exited 0. Every response retained
source: "oauth", primary usage 10%, weekly usage 20%, and a currentusage.updatedAt. The recent cache attaches in both binaries. Only the patched CLI omits the 65-day-old and future-dated cache. All six runs preserved the cache and settings byte-for-byte.Reproduction fixture (synthetic data only, macOS)
Build the PR CLI with
swift build --product CodexBarCLI, retain the normal installed 0.83.0 app, then run this Python fixture from the repository root:Proposed cutoff for maintainer review
The requested bug fix permits a bounded age policy; this patch proposes 15 minutes as that default. A supplemental dashboard supplies mutable balances and reset limits, so an ownership match alone is insufficient to present it beside a newly measured quota. The optional supplement can be omitted without weakening the fresh usage result, triggering a web login, or deleting historical cache data. The bound is checked against attachment time rather than the usage timestamp, so an old usage measurement cannot make an old cache appear fresh. Timestamps up to 60 seconds ahead are tolerated for clock skew; those further ahead fail closed.
This is the author's concrete policy proposal for maintainer review, not a claim that upstream previously approved this cutoff. Maintainers retain the normal decision on accepting that default; no merge is requested by this validation handoff.
Earlier CI and review state (c706263, before clock-skew allowance)
CI run 41278764208 passed on
c7062634745db4be16f82e33baff5821361b144b: full macOS tests, Linux x64/ARM64 builds/tests/smoke checks, musl build/smoke check, lint, and aggregate gate. GitGuardian passed; main-only cache pruning was skipped normally.ClawSweeper review 2 found no actionable code defects and accepted the terminal/upgrade proof as sufficient. Its sole remaining item is maintainer approval of the proposed 15-minute default; the PR is labeled
proof: sufficientandstatus: ⏳ waiting on maintainer. No maintainer approval is claimed.There are no inline review threads. GitHub reports a clean, mergeable PR; upstream main remains
897bff31731bb4b21b105f6e8bf4c32a8a8bf095, included in this branch. No merge performed.Review fixes (2026-11-01)
now(for example after a small NTP correction) now attach. Caches stamped further ahead are still dropped. Before this, any future timestamp was rejected.openAIDashboardCacheMaxAge(15 min) andopenAIDashboardCacheFutureTolerance(60 s).import Foundationreorder.#4882. It conflicts textually with fix(dashboard): flag Claude's unavailable model breakdown as a warning #4877's### Fixedbullet; resolve by keeping both.Tests:
CLIOpenAIDashboardCacheTests, 12 passed. The parameterised age test now covers-61and-3600(dropped) and-60(attached).Real behavior proof
Current-head native macOS CLI proof at
eeb0e1d65941346a132b69b6a726c11f4541cde7(documentation-only follow-up tob14654258). Built this worktree withswift build --product CodexBarCLIand invoked the actual executable twice throughusage --provider codex --source oauth --jsonin fresh processes. No production code or command output was injected.Each run uses an isolated synthetic home (
HOMEandCFFIXED_USER_HOMEaligned), synthetic Codex auth/config, and an ownership-compatible dashboard cache.chatgpt_base_urlroutes OAuth to a loopback fixture returning fresh 10% / 20% usage; optional reset-credit requests return 404. Keychain access is disabled and browser cookies are off. No real accounts or remote provider probes are involved. Identity fields are redacted below; no emails, account IDs, tokens, IP addresses, or personal paths are published. JSON is pretty-printed with all non-identity CLI fields preserved.The first cache is seeded 30 seconds ahead and attaches; the second is seeded 120 seconds ahead and
openaiDashboardis absent. The recorded timestamp differences include process startup time. SHA-256 assertions verify dashboard cache, settings, Codex config, and auth remain byte-identical after each run. Both commands exit 0 with fresh primary/weekly usage.Validation: native CLI build and both synthetic integration cases passed. Existing fixed-clock
CLIOpenAIDashboardCacheTestsalready cover the inclusive 60-second allowance and rejection at 61 seconds, so no additional unit cases were needed. Localmake test-fast FILTER=CLIOpenAIDashboardCacheTestscannot build because this CLT-only Mac lacks Xcode'sactool; exact-head macOS CI remains the unit-test gate.make checkpassed with Python 3.13 andTOOLCHAIN_DIR=/Library/Developer/CommandLineTools, including SwiftFormat and strict SwiftLint (zero violations).Current-head CI run 41355639931 passed on
eeb0e1d65941346a132b69b6a726c11f4541cde7: macOS tests and plugin checks, Linux x64/ARM64 builds/tests/smoke checks, musl build/smoke checks, lint, and the aggregate gate. GitGuardian passed; main-only cache pruning was skipped normally.gh pr checks --watchcompleted successfully.ClawSweeper's current-head re-review accepts the shipped-CLI proof as sufficient and reports no actionable findings. Its remaining items are maintainer action/decision on the bounds.
The earlier c706263 trace above remains historical upgrade/stale-cache evidence; this current-head trace supplies the previously missing clock-skew proof. The proposed freshness bounds still require steipete's owner decision; no approval or resolution is claimed.
🤖 Generated with Claude Code