fix(codex): exclude queue waiting from catch-up delays - #3566
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: needs maintainer review before merge. Reviewed September 11, 2026, 8:22 PM ET / September 12, 2026, 00:22 UTC (Revision 3). ClawSweeper reviewWhat this changesMeasures Codex history scans inside the shared executor and uses their active duration to schedule subsequent usage and Spend Dashboard catch-up passes, with regression coverage and documentation. Merge readiness✅ Ready for maintainer review This remains a useful, focused fix: current main and the latest release still count queue waiting toward catch-up delays. No blocking defect was found, and the supplied production-executor trace supports the correction. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local session history on a shared serial queue to calculate usage and spend. Background workers use scan duration and power conditions to schedule further work before publishing completed history. flowchart TD
A[Local Codex history] --> B[Shared serial scan queue]
B --> C[Measure executing scan]
C --> D[Cache and progress status]
C --> E[Active duration]
E --> F[Power and thermal scheduling policy]
F --> B
D --> G[Usage and Spend Dashboard publication]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep scan-duration accounting inside the executor while retaining existing resource safeguards and complete-history publication, with broader discovery and freshness work tracked separately. Do we have a high-confidence way to reproduce the issue? Yes: current-main workers time the queued await, and the unchanged AC policy multiplies that duration by 999. Queue contention therefore creates excess sleep; this review established the mechanism from source without executing tests. Is this the best way to solve the issue? Yes: measuring inside the existing executor is the narrowest shared correction, and both consumers use it without altering cancellation, settings, scan budgets, or publication rules. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 830cdd86c4b6. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
|
Verified head The final full local run passed all 1,072 selections in 90 groups, with zero failures, retries, or timeouts. The targeted scheduler/executor tests and 41 architecture-gate tests pass; existing provider memberships and fingerprints are unchanged. Make check and final independent P0–P2 branch review pass. A separately built integration executable used the production serial executor and unchanged scheduling policy while parsing 10,000 synthetic JSON records behind another queued task. Its output was: The old-delay value feeds the measured total await into the existing policy, matching the old workers' input; the fixed value uses the executor's own measured work. A freshly built Developer ID–signed app also passed native menu interaction in the isolated synthetic smoke mode. Ready for squash. This corrects queue-wait debt only: retain the initial automatic delay, resource safeguards, and complete-history publication. Leave #3508 and #3411 open, and keep #3509 held. The release note is in #3567. |
Collect the ordered 0.59.1 Unreleased notes and contributor credits after #3539, #3566, #3547, #3525 and #3544. Preserve all released sections. Separate process-fixture startup from measured cleanup deadlines and use the existing task-local authentication hook in Claude retry fixtures. Retain retry, timeout, ownership, and dedicated authentication tests. Dependency pins remain current within their existing policies; larger migrations remain separate.
Automatic Codex history catch-up currently counts time waiting behind another account or provider on the shared serial scan queue as active work. Its duty-cycle policy then multiplies that wait into additional sleep. Measure the scan inside the executor and use its own active duration in both usage and Spend Dashboard workers.
Preserve the existing power/thermal policy, initial delay, scan budgets, cancellation, and complete-history publication boundary. Refs #3508 and #3411; these broader reports remain open. This does not change held PR #3509 or enable its dormant accelerated path. The changelog entry will be centralized in the final batch notes PR.
Validation so far: 48 focused regression tests passed, independent P0–P2 review clean, and a real integration executable using the production executor and policy parsed 10,000 synthetic JSON records. A 460 ms total await contained only 32 ms of active work: the old delay was 459 seconds, while the corrected delay was 32 seconds. The final full local suite passed 1,072 selections in 90 groups with zero failures or retries. Signed native menu smoke, make check, 41 architecture-gate tests, and final branch review pass. Exact-head CI is green: https://github.com/steipete/CodexBar/actions/runs/34658785497.