fix(codex): keep pending migration paths ahead of reseeded files - #3548
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
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. Reviewed September 10, 2026, 7:59 PM ET / 23:59 UTC. ClawSweeper reviewWhat this changesPreserve waiting Codex history files ahead of reseeded files so bounded spend scans advance, while retaining compatibility with existing caches. Merge readiness⛔ Blocked before merge - 2 items remain This remains a useful, focused fix: current main and v0.58.0 retain the starvation mechanism, and the related merged fixes address different paths. No functional defect was found in the patch; release-cache adoption coverage needs one narrow extension. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local Codex session files into a SQLite cost cache. Its bounded work queue controls catch-up progress, and inventory validation determines when spend reports can be published. flowchart LR
A[Local session history] --> B[Migration requirements]
C[Persisted pending queue] --> D[Preserve waiting files first]
B --> D
D --> E[Bounded scanner]
E --> F[SQLite cost cache]
F --> G[Inventory validation]
G --> H[Spend reports]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Keep the queue-order repair and demonstrate that a populated v0.58.0 cache adopts the new fingerprint without rebuilding or losing resumable work. Do we have a high-confidence way to reproduce the issue? Yes, source establishes a concrete path: repeated full-inventory migration requirements prepend the same newest files before a 512-candidate scan, starving the stale tail. The contributor reports a failing regression and real-cache observations; this review did not execute them. Is this the best way to solve the issue? Yes, preserving existing queue order is the narrowest repair and fits the current fairness and exact-validation mechanisms. The cache-adoption test should also exercise the newly added predecessor. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 9f4f544a5bf8. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What 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
|
Repeated full-inventory reseeds must not repeatedly place completed files ahead of the stale tail. Preserve existing waiters while retaining newest-first initial seeding and exact completion validation. Test adoption of the released native parser fingerprint and document the distinct Pi/OMP reparse behavior. Related to steipete#3411 and steipete#3548. Co-authored-by: Nikolenko.Sergei <Nikolenko.Sergei@icloud.com>
70c22fe to
69da567
Compare
Repeated full-inventory reseeds must not repeatedly place completed files ahead of the stale tail. Preserve existing waiters while retaining newest-first initial seeding and exact completion validation. Test adoption of the released native parser fingerprint and document the distinct Pi/OMP reparse behavior. Related to steipete#3411 and steipete#3548. Co-authored-by: Nikolenko.Sergei <Nikolenko.Sergei@icloud.com>
69da567 to
6f5fb9a
Compare
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(codex): keep pending migration paths ahead of reseeded files This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
Honor Hide Personal Info in Codex account switcher labels and tooltips. Remove embedded workspace emails, keep distinct numbered labels when space is limited, and preserve account numbers through managed-to-live promotion using persisted slot identity. Normal selection remains unchanged. Includes source extraction, reuse of existing test seams, model/projection and native UI regressions, provider docs and the coordinated 0.59.1 changelog for #3551 and #3548. Co-authored-by: Chandler Anderson <chanderson90@proton.me>
|
Landed as 51d32c4ac31. Existing waiting history paths now stay ahead of full-inventory revisits, preventing the same completed prefix from starving older files. Initial newest-first ordering and exact completion validation remain unchanged. Thanks @SergeiNikolenko. Verification used synthetic histories and isolated test stores: swift test --filter 'CostUsageBoundedProgressTests|CostUsageCacheWideMigrationTests|CostUsageCatchUpCompletionTests|CostUsageStoreTests|CostUsageScannerWhitespaceTests|CodexSubagentOrdinalBoundaryTests|CostUsagePerformanceGateTests|CostUsageStoreReadWorkTests|PiSessionCostCompatibilityTests'
make check
make test
Scripts/regenerate-codex-parser-hash.sh checkThe commands ran through the credential/session-isolating test wrapper. The regression failed three assertions with the original ordering; the repaired focused run passed 192 tests. The full suite passed all 1,066 selections / 89 groups on the first attempt, with no retries or timeouts. Independent P0–P2 review was clean, and exact-head CI passed both macOS shards and all Linux builds. Native predecessor cache adoption is covered; Pi/OMP retains its documented one-time reparse. The coordinated changelog is in #3551 under 0.59.1 Unreleased. The broader #3411 report remains open. |
Fixes bounded Codex history catch-up repeatedly revisiting the same completed prefix while older files remain untouched. When pricing or priority metadata repeatedly reseeds the full inventory, existing waiting paths now stay ahead of newly requested revisits. Initial newest-first seeding, normalization, deduplication and exact completion validation are preserved.
The maintainer pass adds explicit adoption coverage for released native parser fingerprint
4969a789db679c93, reconciles the existing partially drained queue test with the fairness rule, and includes the previously omitted migration/completion suites. The generated fingerprint isf5fdba377006d7be. Native stored rows, checkpoints and reports are preserved; Pi/OMP retains its documented one-time reparse on a parser-hash change.Validation:
make test: all 1,066 selections across 89 groups passed first time, with no retries or timeouts (1104.0 seconds).make check, parser-hash check and independent P0–P2 review passed.Provider docs are updated. The 0.59.1 changelog entry is recorded in companion #3551. Related to #3411. Thanks @SergeiNikolenko!