Skip to content

fix(codex): keep pending migration paths ahead of reseeded files - #3548

Merged
steipete merged 1 commit into
steipete:mainfrom
SergeiNikolenko:fix/migration-queue-reseed-order
Sep 11, 2026
Merged

fix(codex): keep pending migration paths ahead of reseeded files#3548
steipete merged 1 commit into
steipete:mainfrom
SergeiNikolenko:fix/migration-queue-reseed-order

Conversation

@SergeiNikolenko

@SergeiNikolenko SergeiNikolenko commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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 is f5fdba377006d7be. Native stored rows, checkpoints and reports are preserved; Pi/OMP retains its documented one-time reparse on a parser-hash change.

Validation:

  • Three assertions failed against seed-first ordering: the untouched 88-file tail never progressed, and a discovered waiter was displaced. Both pass after the repair.
  • 192 focused tests across nine suites passed, including initial seeding, cache-wide migration, catch-up completion, exact predecessor adoption, parser boundaries, performance/read-work and Pi compatibility.
  • Full 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.
  • Proof uses synthetic histories and test stores; no personal cache was opened. Final-head CI passed all checks.

Provider docs are updated. The 0.59.1 changelog entry is recorded in companion #3551. Related to #3411. Thanks @SergeiNikolenko!

@clawsweeper

clawsweeper Bot commented Sep 10, 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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T00:00:20.713324Z 70c22fe PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@clawsweeper clawsweeper Bot added 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. proof: sufficient Contributor real behavior proof is sufficient. 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 11, 2026
@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 10, 2026, 7:59 PM ET / 23:59 UTC.

ClawSweeper review

What this changes

Preserve 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
Reviewed head: 70c22fe058efb1c604414d7e83e4fdf87f5a6da6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with concrete real-cache progress evidence and useful regression coverage, pending a small release-cache adoption check.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.
Evidence reviewed 9 items Current-main starvation mechanism: Main places migration seed paths before existing pending paths. Repeated pricing or priority changes can reseed the full inventory, while candidate selection takes only the bounded prefix and finalization removes completed entries.
Released behavior remains affected: The v0.58.0 scanner retains seed-first ordering, and its generated fingerprint is 4969a789db679c93. The requested ordering repair is therefore neither present on the reviewed main nor in the supplied latest release.
Introduced repair and regression coverage: The verified base-to-head patch reverses only the two queue-assembly loops, retaining normalization and deduplication. The new 600-file regression changes migration requirements between passes, checks that all 88 stale tail files are reached, and subsequently checks exact completion.
Findings None None.
Security None None.

How this fits together

CodexBar 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]
Loading

Before merge

  • Resolve merge risk (P1) - The new parser fingerprint changes the cache generation, but the submitted adoption test never opens a cache using the newly accepted v0.58.0 fingerprint. Preservation of that release's rows and checkpoints is therefore not explicitly validated.
  • Complete next step (P2) - Add the v0.58.0 fingerprint to the adoption test's arguments, verify preserved snapshots and checkpoints with zero rebuilds, and complete make check and make test.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +10/-3; tests +68/-0 The small queue-order and compatibility change has focused regression coverage; production growth consists primarily of explanation and the predecessor entry.

Merge-risk options

Maintainer options:

  1. Exercise the released cache fingerprint (recommended)
    Add 4969a789db679c93 to the existing parameterized adoption regression and verify preserved snapshots, checkpoints, and zero rebuilds.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Add 4969a789db679c93 to the arguments of the existing compatible-predecessor adoption test, preserving its snapshot, checkpoint, metadata, and zero-rebuild assertions; run focused store and bounded-progress tests, make check, and make test without live account or Keychain access.

Technical review

Best 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.

Labels

Label changes:

  • add P2: Large Codex histories can remain stuck in spend catch-up, while the reported CLI path still returns history.
  • add merge-risk: 🚨 compatibility: The parser fingerprint changes the SQLite cache generation, and the newly supported released predecessor lacks direct adoption-test execution.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.

Label justifications:

  • P2: Large Codex histories can remain stuck in spend catch-up, while the reported CLI path still returns history.
  • merge-risk: 🚨 compatibility: The parser fingerprint changes the SQLite cache generation, and the newly supported released predecessor lacks direct adoption-test execution.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured real-cache run reports the changed bounded scanner progressing through stale files on an 8,246-file history after repeated pre-fix stalls. This supports the queue-order repair; release-fingerprint preservation remains a separate upgrade-validation gap.

Evidence

Acceptance criteria:

  • [P1] swift test --filter 'CostUsageStoreTests|CostUsageBoundedProgressTests'.
  • [P1] make check.
  • [P1] make test.

What I checked:

  • Current-main starvation mechanism: Main places migration seed paths before existing pending paths. Repeated pricing or priority changes can reseed the full inventory, while candidate selection takes only the bounded prefix and finalization removes completed entries. (Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift:3073, 9f4f544a5bf8)
  • Released behavior remains affected: The v0.58.0 scanner retains seed-first ordering, and its generated fingerprint is 4969a789db679c93. The requested ordering repair is therefore neither present on the reviewed main nor in the supplied latest release. (Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift:3073, 88fa2f45fa1e)
  • Introduced repair and regression coverage: The verified base-to-head patch reverses only the two queue-assembly loops, retaining normalization and deduplication. The new 600-file regression changes migration requirements between passes, checks that all 88 stale tail files are reached, and subsequently checks exact completion. (Tests/CodexBarTests/CostUsageBoundedProgressTests.swift:1110, 70c22fe058ef)
  • Real-cache progress evidence: The captured PR body, sourceRevision 4fb3f952d869a8e8d2dfea0d774def17940ea72c3dd76bc7442f615f445cb137, reports bounded scanner passes over a copied 287 MB SQLite cache and 8,246 real history files. Before the fix, three passes repeatedly attempted 512 files with 7,734 pending; afterward, passes parsed 23, 26, and 7 stale files and reduced the queue. This supports restored progress, without establishing final dashboard publication or fingerprint-adoption preservation. (70c22fe058ef)
  • Upgrade test does not exercise the new predecessor: The added fingerprint appears in the expected compatibility set at line 1037, but not in the @test arguments beginning at line 1008. The existing test already verifies snapshot equality, zero rebuilds, metadata adoption, and resumable checkpoints, making another argument the narrow validation repair. (Tests/CodexBarTests/CostUsageStoreTests.swift:1037, 70c22fe058ef)
  • Compatibility implementation: The parser fingerprint contributes to SQLite user_version. The existing adoption path checks the predecessor version and database integrity, then updates metadata transactionally; unsupported versions can trigger rebuilding. No row format, schema DDL, authorization, or account boundary is changed by this patch. (Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore.swift:602, 70c22fe058ef)

Likely related people:

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

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Exercise fingerprint 4969a789db679c93 in the existing adoption regression and record successful preservation and zero-rebuild assertions.

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 added a commit to SergeiNikolenko/CodexBar that referenced this pull request Sep 11, 2026
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>
@steipete
steipete force-pushed the fix/migration-queue-reseed-order branch from 70c22fe to 69da567 Compare September 11, 2026 06:05
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>
@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown

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.

@steipete
steipete merged commit 51d32c4 into steipete:main Sep 11, 2026
9 checks passed
steipete pushed a commit that referenced this pull request Sep 11, 2026
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>
@steipete

Copy link
Copy Markdown
Owner

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 check

The 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.

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. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. 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.

2 participants