refactor(server): share scheduling for tasks and limit recovery - #12795
juliusmarminge merged 6 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between f5cc6ed and f34dc5e3d9b85e2c21355441a1132ceb450dcd30. 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe PR adds a shared Effect scheduler with scoped registrations, serialized execution, failure handling, and five-second polling. Scheduled-task and usage-limit recovery services use it. The projection store supplies filtered recovery candidates. Tests cover concurrency, cleanup, restart, and recovery behavior. ChangesShared scheduler adoption
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant ScheduledTaskService
participant UsageLimitRecoveryService
participant ProjectionStore
participant OrchestrationService
Scheduler->>ScheduledTaskService: Run registered due-task work
ScheduledTaskService->>OrchestrationService: Dispatch due task
Scheduler->>UsageLimitRecoveryService: Run registered recovery work
UsageLimitRecoveryService->>ProjectionStore: Get limit-recovery candidates
UsageLimitRecoveryService->>OrchestrationService: Dispatch recovery retry
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a new production scheduler and changes the timing, lifecycle, concurrency, and persistence-query behavior of both scheduled tasks and usage-limit recovery. Because it restructures shared background infrastructure and alters existing runtime workflows, human review is warranted. No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
f5cc6ed to
bbf610b
Compare
bbf610b to
f624664
Compare
f624664 to
f34dc5e
Compare
f34dc5e to
47a7d48
Compare
57dc0ac to
62c0cf7
Compare
|
Effect Service Conventions found one blocking issue; see the inline review comment for the required namespace-import fix. Posted via Macroscope — Effect Service Conventions |
|
Effect Service Conventions found one blocking issue; see the inline review comment for the required exported service constructor fix. Posted via Macroscope — Effect Service Conventions |
62c0cf7 to
3b70bfc
Compare
3b70bfc to
4dc66ff
Compare
61a3501
into
t3code/codex-turn-mapping
Limit retries added a separate 30-second polling loop alongside Scheduled Tasks. Extract the existing five-second task scheduler into one shared server primitive and register both features with it.
The scheduler checks due work on registration, then every five seconds. It owns source lifetimes, error isolation, and preventing overlapping sweeps. Slow task work does not delay limit recovery. Each feature keeps its existing persisted state and dispatch guards, so retries survive restarts without becoming user-visible Scheduled Tasks. Limit recovery uses one focused SQLite query for unarmed failures affected by preferences and due enabled retries. Future, cancelled, archived, settled, and pending-request threads are excluded before decoding. Scheduler ticks no longer rebuild shell snapshots, count transcript history, or traverse fork ancestors.
Stacked on #12783.
Verification: 81 focused tests pass, covering the shared scheduler, overdue work after restart, SQL recovery filtering, cancellation, snooze deadlines, provider-session errors, latest root failures, and 15 dispatch guards. Scoped server typecheck and targeted lint pass. SQLite query-plan inspection confirms indexed latest-run/root-error lookups. The query still checks thread metadata every five seconds; CPU usage on large databases has not been benchmarked. No client presentation changes; automated behavior tests provide the evidence.
Model: GPT-6. Harness: Codex.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Integration verification: the shared scheduler constructor is private, fixing the unused export reported by CI. All five scheduler lifecycle and integration tests pass, including immediate overdue recovery, source isolation, interruption, and no overlapping work. Scoped server typecheck passes.
Built with GPT-6 in Codex.