test: add StoreStress harness and contention regression tests - #2766
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5729671d59
ℹ️ 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".
| if Self.isTransientContention(error) { | ||
| return fallback |
There was a problem hiding this comment.
When a transient lock timeout happens during one component of saveCodexCache, this returns the fallback for that single write, but callers such as persistFile and saveCodexCache ignore the false results and continue with later writes like aggregates and metadata. If the lock clears before those later calls, the store can persist lastScan/window metadata and global aggregates for a scan whose per-file rows or snapshots were skipped, so the UI can read an internally inconsistent cache instead of retrying the whole refresh against the previous committed state.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs maintainer review before merge. Reviewed August 8, 2026, 2:37 PM ET / 18:37 UTC. ClawSweeper reviewWhat this changesAdds a separate StoreStress Swift package for SQLite cost-cache crash, contention, WAL, corpus, and descriptor exercises, plus a regression test that verifies a lock timeout preserves the existing database. Merge readinessThe rebuilt branch now contains an independent non-shipping stress harness and a focused lock-contention regression test; the earlier production-policy findings do not apply to this head. It is owner-authored, so it remains open for normal maintainer review and CI completion. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s cost scanner persists Codex session-derived usage in a SQLite cache used by both the menu-bar app and CLI cost commands. The store writes scan results and readers consume snapshots, so lock handling must preserve cached history when another connection is writing. flowchart LR
A[Codex session files] --> B[Cost scanner]
B --> C[SQLite cost cache]
D[CLI cost command] --> C
E[Menu bar app] --> C
C --> F[Usage reports]
G[StoreStress harness] --> C
H[Lock contention test] --> C
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the production recovery policy from current main, land the focused regression and harness after CI, and separately decide whether the save pipeline should abort its remaining writes after a preserved lock failure. Do we have a high-confidence way to reproduce the issue? Yes: the new focused test holds an SQLite write lock beyond the configured busy timeout, then verifies that the blocked write returns false while the existing database survives. The supplied PR body also records a separate lock-holder exercise and sustained WAL-reader run. Is this the best way to solve the issue? Yes for the PR’s stated verification goal: a non-shipping harness plus a focused regression is a narrow way to preserve coverage without changing the established recovery policy. It does not resolve the separate current-main question of whether a multi-write cache save should stop after one preserved failure. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 985546901bb9. 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 (2 earlier review cycles)
|
5729671 to
b333cd1
Compare
steipete
changed the title
clawsweeper
Bot
added
rating: 🐚 platinum hermit
1 participant
Salvage outcome
Current
mainsupersedes this PR's original production fix:BUSY,LOCKED,FULL, constraint, I/O, and other transient/data-shape failures preserve it.This branch was rebuilt on current
main; both duplicate production patches were discarded. What remains is independent value:Scripts/StoreStresspackage for crash, contention, corpus, WAL, and descriptor testing;codexbar costand other CLI cost surfaces instantiateCostUsageFetcher, so app and CLI processes can each open a writable scanner/store connection.No changelog entry is needed for test/review tooling.
Crash recovery
Each cycle used its own temp store. The writer was killed with
SIGKILL; verification reopened SQLite directly, ranPRAGMA integrity_check, and (for normal writes) checked that committed aggregates exactly formed the expected triangular-number prefix.integrity_check=okConcurrency and WAL
One writer and four independent read-only WAL readers ran for 190 seconds. The writer completed 56,009 iterations, 280 retention passes, and 112 budget/vacuum passes with 0 write errors and 0 store rebuilds.
Frozen real-corpus rebuild
~/.codex/sessionswas APFS-cloned into a temp directory and made read-only before measurement. The source corpus and~/Library/Caches/CodexBarwere never written. The frozen corpus contained 1,948 JSONL files and 2,371,674,974 logical bytes. The optimized standalone harness wrote only to a separate temp store./usr/bin/time -land immediate unchanged-corpus results:The second pass confirms zero append/rescan work at corpus scale. Its wall/RSS are not near zero because loading the persisted cache, rediscovering files, and building the report remain nontrivial fixed overhead; the table keeps that distinction explicit.
Verification
swift build -c releaseswift build --package-path Scripts/StoreStress -c release -Xswiftc -enable-testingmake check: passed, 0 lint violationsmake test: 822/822 selections passed in 69/69 groups; 0 retries, 0 timeouts