Skip to content

fix(cost): preserve Pi session caches across Linux refreshes - #3446

Merged
steipete merged 1 commit into
mainfrom
codex/fix-pi-cache-atomic
Sep 6, 2026
Merged

fix(cost): preserve Pi session caches across Linux refreshes#3446
steipete merged 1 commit into
mainfrom
codex/fix-pi-cache-atomic

Conversation

@steipete

@steipete steipete commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Pi-compatible session-cost caching had the same Linux replacement failure fixed for pricing catalogs in #3444: the second save could remove the cache file. A subsequent cached read then lost its saved scan state and totals and needed another scan.

This replaces the remaining temporary-write/replace sequence with one atomic destination write. The schema, cache path, timezone assignment, and best-effort save contract stay intact; an encoding failure also leaves the previous file untouched. Production is net −10 lines (2 added, 12 removed). The change includes a portable regression, an Unreleased changelog entry, and correction of the stale Claude documentation path to the existing v8 cache.

Validation:

  • Linux x86_64 with the official Swift 6.3.3 container: unchanged production code fails on save 2 with the cache file missing; the candidate passes 100 saves with independent disk/readback, scan timestamp, pricing-key, provider-total, timezone, and staging-file checks.
  • All 23 focused Pi cache/scanner/compatibility tests pass.
  • make check passes, and make test passes all 1,026 selections across 86 groups on the first run without retries.
  • CI passes all nine checks, including both macOS shards and Linux x64, arm64, and musl.

This is a follow-up from auditing the remaining production use of FileManager.replaceItemAt; it repairs the existing Pi/OMP history used by Codex and Claude cost reporting.

Follow the pricing-cache repair in #3444 by replacing the remaining redundant file swap with one atomic write. Preserve complete scan, pricing, timezone and provider state across repeated saves, and retain the best-effort persistence contract.
@clawsweeper

clawsweeper Bot commented Sep 6, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 6, 2026
@clawsweeper

clawsweeper Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 5:09 AM ET / 09:09 UTC.

ClawSweeper review

What this changes

The PR makes Pi and OMP cost-cache saves use a single atomic write, adds repeated-save regression coverage, and updates cache documentation and release notes.

Merge readiness

Ready for maintainer review

This remains a useful, focused fix: current main and v0.56.6 retain the affected replacement sequence. No introduced correctness or security defect was found.

Priority: P2
Reviewed head: e5e3a31a0f0b6ae760111b3d72483e009e391139

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A narrow, maintainable repair with focused disk-persistence coverage and no identified blocking defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The OWNER-authored PR is exempt from the contributor proof gate and changes no authority boundary; its body reports Linux before/after execution of the production cache writer with repeated disk readback.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The OWNER-authored PR is exempt from the contributor proof gate and changes no authority boundary; its body reports Linux before/after execution of the production cache writer with repeated disk readback.
Evidence reviewed 7 items Reviewed production change: The complete introduced diff replaces the two-stage file swap with an encoding guard and atomic destination write. Cache version, destination, serialized fields, timezone assignment, and best-effort return behavior remain unchanged.
Still necessary on main: The pinned current main still writes a temporary file and calls replaceItemAt when the destination exists.
Latest release retains old writer: The v0.56.6 source contains the same two-stage replacement sequence; this Pi-cache repair is not present in the supplied latest release.
Findings None None.
Security None None.

How this fits together

CodexBar scans Pi and OMP session logs into cached usage totals and scan checkpoints. Codex and Claude cost reports reuse that cache between scans.

flowchart LR
  A[Pi and OMP session logs] --> B[Session cost scanner]
  B --> C[Usage totals and scan state]
  C --> D[Atomic cache write]
  D --> E[Version 8 cache file]
  E --> F[Cached report reader]
  F --> G[Codex and Claude cost reports]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production −10 lines; tests +34 lines The repair reduces persistence logic and adds focused regression coverage without expanding the production surface.

Technical review

Best possible solution:

Retain the existing version-8 cache contract while using one atomic destination write for both creation and refresh.

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

Yes: repeatedly saving through the production Pi cache writer on Linux is a concrete reproduction path, and the body reports failure on save two before the fix. This read-only review inspected that path without executing it.

Is this the best way to solve the issue?

Yes: the single atomic write removes the redundant replacement step while preserving the existing serialized contract and failure behavior; no migration or competing implementation is introduced.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 5e69a92232d8.

Labels

Label changes:

  • add P2: This repairs Linux cost-cache refresh persistence with a limited blast radius and recoverable source logs.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The OWNER-authored PR is exempt from the contributor proof gate and changes no authority boundary; its body reports Linux before/after execution of the production cache writer with repeated disk readback.

Label justifications:

  • P2: This repairs Linux cost-cache refresh persistence with a limited blast radius and recoverable source logs.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The OWNER-authored PR is exempt from the contributor proof gate and changes no authority boundary; its body reports Linux before/after execution of the production cache writer with repeated disk readback.

Evidence

What I checked:

  • Reviewed production change: The complete introduced diff replaces the two-stage file swap with an encoding guard and atomic destination write. Cache version, destination, serialized fields, timezone assignment, and best-effort return behavior remain unchanged. (Sources/CodexBarCore/PiSessionCostCache.swift:39, e5e3a31a0f0b)
  • Still necessary on main: The pinned current main still writes a temporary file and calls replaceItemAt when the destination exists. (Sources/CodexBarCore/PiSessionCostCache.swift:45, 5e69a92232d8)
  • Latest release retains old writer: The v0.56.6 source contains the same two-stage replacement sequence; this Pi-cache repair is not present in the supplied latest release. (Sources/CodexBarCore/PiSessionCostCache.swift:45, 1696c7a71c94)
  • Regression and compatibility coverage: The new portable test creates a fresh cache and performs ten saves, independently decoding disk contents and checking timestamps, pricing keys, totals, timezone, load behavior, and staging-file cleanup. Existing compatibility tests cover retained file cursors, contributions, entry usage, and independent invalidation. Package.swift includes TestsLinux in its unconditional test targets. (TestsLinux/PiSessionCostCacheReplacementTests.swift:15, e5e3a31a0f0b)
  • Observable cache consumer: The scanner persists completed scan state and rejects missing or incompatible caches on cached reads; CostUsageFetcher merges the resulting Pi usage into provider-specific cost reports. (Sources/CodexBarCore/PiSessionCostScanner.swift:187, e5e3a31a0f0b)
  • Reported validation and related repair: The captured PR body reports Linux x86_64/Swift 6.3.3 failure on the unchanged writer's second save and 100 successful candidate saves with disk readback, plus 23 focused tests, make check, and 1,026 full-suite selections. These are contributor-reported results, not reviewer execution. The supplied GitHub context verifies fix(pricing): atomically replace cached catalogs on Linux #3444 merged the analogous pricing-catalog repair, which does not repair this separate Pi writer. (e5e3a31a0f0b)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Milan Mijatovic: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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.

@steipete
steipete merged commit c15f736 into main Sep 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant