Skip to content

Keep cached spend visible while Codex refreshes - #2628

Merged
steipete merged 3 commits into
mainfrom
codex/cached-spend-ux
Aug 3, 2026
Merged

Keep cached spend visible while Codex refreshes#2628
steipete merged 3 commits into
mainfrom
codex/cached-spend-ux

Conversation

@steipete

@steipete steipete commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • surface the post-fix: preserve fork accuracy during bounded Codex cost catch-up #2525 scanner's persisted Codex report before the live Usage & Spend load finishes
  • keep cached rows account-scoped and reject auth-fingerprint changes during hydration
  • retain totals until the authoritative scan converges, while showing refresh activity in the dashboard and menu Cost row
  • reuse the ambient scanner cache for the live-system account; managed/profile accounts keep distinct cache identities

This carries forward @hhh2210's retained-totals UX from #2397 without porting that PR's pre-#2525 cache or controller state machine. Reference: #2397.

Visual proof

Idle retained totals Same totals while refreshing
Idle retained cost totals Refreshing retained cost totals

The fixtures are synthetic and rendered through the real SwiftUI cost-row view. The dashboard keeps its existing stale-data label and catch-up panel.

Proof

  • make check
  • make test — 804 selections in 67 groups; 0 failed groups, retries, or timeouts
  • focused cached-presentation and menu model suites — 9 tests passed
  • screenshot render proof — 1 test passed; generated artifacts visually inspected and hash-verified
  • Codex autoreview — clean for the implementation/test bundle and the screenshot harness; no accepted/actionable findings

The Codex reviewer cannot ingest opaque PNG diff hunks, so the generated binaries were validated separately through the source-blind artifact check above.

Peter Steinberger and others added 2 commits August 3, 2026 14:56
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 3, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0eb1c0690b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +569 to +570
if request.source == .liveSystem {
return costUsageDirectory.deletingLastPathComponent()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep live cache keyed by Codex auth

When the live-system Codex account rotates in the same CODEX_HOME, this now points every live account at the shared ambient cache root instead of the cacheIdentity that includes the auth fingerprint. The cached dashboard primer only verifies the current auth before reading, but the persisted cost cache is not stamped with the auth that wrote it, so a report produced under the previous live account can be rehydrated under the newly signed-in account while the scan catches up. Keep the live cache account-scoped, or add an auth-stamp check before reusing the ambient cache.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Aug 3, 2026
@clawsweeper

clawsweeper Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 3, 2026, 6:32 PM ET / 22:32 UTC.

ClawSweeper review

What this changes

The branch shows persisted Codex spend totals immediately in the Usage & Spend dashboard and menu, with a refresh indicator until the live scan finishes.

Merge readiness

Blocked by patch quality or review findings - 6 items remain

Keep open, but do not merge until cached live-system spend is tied to the account that wrote it; the current auth checks cannot distinguish a cache written before an account rotation.

Priority: P1
Reviewed head: 32a12cff2b684e4f9c5ac7a965c359e791153656

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The proposed UX is visibly demonstrated, but a high-confidence auth-boundary defect makes the patch unready to merge.
Proof confidence 🌊 off-meta tidepool Not applicable: This maintainer-authored PR is exempt from the external proof gate; the prepared screenshots do directly show the menu refresh indicator, but they do not resolve the account-provenance defect.
Patch quality 🧂 unranked krab (1/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This maintainer-authored PR is exempt from the external proof gate; the prepared screenshots do directly show the menu refresh indicator, but they do not resolve the account-provenance defect.
Evidence reviewed 4 items Shared live cache bypasses the auth-scoped identity: The branch routes live-system accounts to the ambient cache root rather than the request's cache identity, while managed and profile accounts remain scoped.
Cache validation has no writer-auth provenance: The persisted cache records scan and root metadata but no Codex auth fingerprint; its root fingerprint is based on session-root paths, which remain unchanged after an in-place account rotation.
Existing auth test covers only an in-flight rotation: The added test changes the auth file during a single cache load, but does not write a cache under account A and then construct a fresh request for account B using the same live home.
Findings 1 actionable finding [P1] Keep the live cache keyed to its auth fingerprint
Security Needs attention Shared cache lacks account provenance: The live-system cache bypasses the auth-inclusive identity, while persisted cache validation tracks roots and scan metadata rather than the writer's auth fingerprint.

How this fits together

CodexBar scans local Codex sessions into a persisted cost cache, then renders those totals in the Usage & Spend dashboard and menu. This branch adds a cache-first display before the normal scanner refresh replaces it.

flowchart LR
A[Local Codex sessions] --> B[Persisted cost cache]
B --> C[Cached dashboard load]
C --> D{Cache belongs to active account?}
D -->|Yes| E[Dashboard and menu totals]
D -->|No| F[Wait for live scan]
A --> G[Authoritative scanner]
G --> E
Loading

Before merge

  • Keep the live cache keyed to its auth fingerprint (P1) - The live-system path now reads the shared ambient cache. After an in-place rotation, a new request carries account B's fingerprint, so both surrounding checks pass while the cache can still contain account A's report; the cache schema has no writer-auth field. Keep this root auth-scoped or stamp and verify writer provenance, then cover write-under-A / hydrate-under-B.
  • Resolve security concern: Shared cache lacks account provenance - The live-system cache bypasses the auth-inclusive identity, while persisted cache validation tracks roots and scan metadata rather than the writer's auth fingerprint.
  • Resolve merge risk (P1) - An existing user who signs out and into another Codex account in the same home can temporarily see the prior account's cached spend until scanning completes.
  • Complete next step (P2) - The remaining blocker is a narrow, source-proven cache-provenance repair with a focused regression path.
  • Improve patch quality - Restore auth-scoped cache provenance or add and verify a writer-auth stamp.
  • Improve patch quality - Add a regression that writes under one auth fingerprint and hydrates after a fresh request observes a different fingerprint.

Findings

  • [P1] Keep the live cache keyed to its auth fingerprint — Sources/CodexBar/SpendDashboardController.swift:569-571
  • [medium] Shared cache lacks account provenance — Sources/CodexBar/SpendDashboardController.swift:570
Agent review details

Security

Needs attention: The cache-first path can expose one local Codex account's spend totals after another account replaces its auth file in the same home.

Review metrics

Metric Value Why it matters
Production vs. test delta production +225/-28, tests +350 The branch adds substantial cache-presentation coverage, but the account-rotation scenario remains untested.

Merge-risk options

Maintainer options:

  1. Restore auth-scoped live caching (recommended)
    Use the existing auth-inclusive cache identity for the live-system path and add an account-A-to-account-B regression before merge.
  2. Authenticate shared-cache provenance
    Keep the ambient cache only if its persisted report carries a writer auth fingerprint that is checked before cached totals are displayed.

Technical review

Best possible solution:

Keep the retained-totals UX, but either retain the auth-inclusive cache identity for live-system accounts or persist and verify the cache writer's auth fingerprint before hydration.

Do we have a high-confidence way to reproduce the issue?

Yes—source establishes a clear path: write the ambient cache under account A, rotate the auth file to account B in the same home, then begin dashboard hydration before the live scan completes.

Is this the best way to solve the issue?

No; the cache-first design is viable, but the shared live cache must retain or verify writer-auth provenance before it can safely present totals.

Full review comments:

  • [P1] Keep the live cache keyed to its auth fingerprint — Sources/CodexBar/SpendDashboardController.swift:569-571
    The live-system path now reads the shared ambient cache. After an in-place rotation, a new request carries account B's fingerprint, so both surrounding checks pass while the cache can still contain account A's report; the cache schema has no writer-auth field. Keep this root auth-scoped or stamp and verify writer provenance, then cover write-under-A / hydrate-under-B.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 47e1d0426236.

Labels

Label justifications:

  • P1: A live Codex account rotation can display another account's cached spend in a normal provider workflow.
  • merge-risk: 🚨 compatibility: The new live cache-root rule changes persisted-cache behavior for existing users who rotate accounts in place.
  • merge-risk: 🚨 security-boundary: Account-scoped spend data can cross the local authentication boundary during the cache-first window.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This maintainer-authored PR is exempt from the external proof gate; the prepared screenshots do directly show the menu refresh indicator, but they do not resolve the account-provenance defect.

Evidence

Security concerns:

  • [medium] Shared cache lacks account provenance — Sources/CodexBar/SpendDashboardController.swift:570
    The live-system cache bypasses the auth-inclusive identity, while persisted cache validation tracks roots and scan metadata rather than the writer's auth fingerprint.
    Confidence: 0.98

Acceptance criteria:

  • [P1] swift test --filter SpendDashboardCachedPresentationTests.
  • [P1] make test.
  • [P1] make check.

What I checked:

Likely related people:

  • steipete: Current cache-scope lines are attributed to Peter Steinberger in git blame, and the current branch implements this cache-first path. (role: recent area contributor; confidence: high; commits: 15630025f298; files: Sources/CodexBar/SpendDashboardController.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageCache.swift)
  • Alec Gutman, Chip: Introduced the unified Usage & Spend dashboard that owns the presentation flow. (role: original dashboard contributor; confidence: medium; commits: 0397529ae6e5; files: Sources/CodexBar/SpendDashboardController.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-03T22:10:32.424Z sha 0eb1c06 :: found issues before merge. :: [P1] Keep the live cache tied to the auth fingerprint

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. P1 Urgent regression or broken agent/channel workflow affecting real users now. and removed proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P2 Normal priority bug or improvement with limited blast radius. labels Aug 3, 2026
@steipete
steipete merged commit 698c332 into main Aug 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant