feat(server): resume unavailable provider sessions with mobile-safe status - #10357
feat(server): resume unavailable provider sessions with mobile-safe status#10357rix1337 wants to merge 1 commit into
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a large cross-layer automatic-resume feature that is enabled by default and can initiate persisted, real provider continuation attempts after failures. Unresolved substantive concerns about provider retry sequencing and error classification also require human verification of the rollout behavior. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 235784dbca
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const awaitsTrailingRuntimeError = | ||
| completedState === "failed" && event.provider === ProviderDriverKind.make("opencode"); |
There was a problem hiding this comment.
Preserve Grok retries after failed turn completion
When a scheduled continuation hits Grok's usage limit again, GrokAdapter.sendTurn emits turn.completed(failed) while unwinding the rejected prompt (GrokAdapter.ts lines 679-690 and 1844-1850), rather than emitting a trailing runtime.error. Because this condition exempts only OpenCode, ingestion can cancel the in-flight resume before ProviderCommandReactor handles the rejected sendTurn; that handler then observes usageLimitResume === null and returns without scheduling the next backoff attempt. Keep the Grok rejection path pending until the reactor classifies the send failure, or otherwise coordinate these terminal events.
AGENTS.md reference: AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 842e2ef. A failed Grok completion no longer cancels an in-flight retry before the reactor handles the rejected sendTurn. The focused test verifies that the completion preserves the pending retry; it does not assume Grok emits a trailing runtime.error.
| import * as Predicate from "effect/Predicate"; | ||
|
|
||
| const USAGE_LIMIT_MESSAGE = | ||
| /(?:usage|rate) limit|spend cap|model (?:is )?(?:currently )?at capacity|quota (?:has been )?(?:exceeded|reached|exhausted)|too many requests|\b(?:404|429)\b|resource[_ ]exhausted|insufficient[_ ]quota/i; |
There was a problem hiding this comment.
Do not treat every HTTP 404 as a usage limit
When a provider returns HTTP 404 for a permanent error such as an invalid endpoint, missing model, or expired session, this regex always classifies it as usage_limit. The adapter and turn-start error paths consequently advertise temporary unavailability and schedule automatic continuation attempts indefinitely instead of surfacing the configuration error. Restrict 404 handling to specific provider responses known to represent transient capacity, while leaving generic 404 failures as provider errors.
AGENTS.md reference: AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 842e2ef. A bare 404 no longer classifies as a usage limit. Explicit temporary service/provider/server unavailability and existing quota/capacity signals still qualify. Tests cover generic 404, missing model, invalid endpoint, expired session, and nested error details.
235784d to
842e2ef
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 842e2ef2f8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| yield* orchestrationEngine.dispatch({ | ||
| type: "thread.usage-limit-resume.schedule", | ||
| commandId: yield* providerCommandId(event, "usage-limit-resume-schedule"), | ||
| threadId: thread.id, | ||
| resumeAt, |
There was a problem hiding this comment.
Exclude pending resumes from automatic settlement
When a provider remains unavailable longer than sidebarAutoSettleAfterDays (three days by default), this schedule does not keep the thread out of automatic settlement: isAutoSettlementCandidate ignores usageLimitResume, while shouldAutoSettleThread ages only user and turn timestamps. The minute-based settlement sweep can therefore settle the thread and the decider cancels its pending resume, which breaks weekly reset times and extended outages despite the documented six-hour retry cadence. Treat a non-null usageLimitResume as active work, or include retry activity in the settlement age.
Useful? React with 👍 / 👎.
What Changed
Builds on #8577 with server-side automatic resume for unfinished provider sessions. It is on by default, with an environment-level opt-out in Settings.
Provider reset times take priority, including dated AM/PM messages, with a one-minute cushion. Otherwise, retries wait 20 minutes three times, one hour five times, then six hours. Pending schedules survive restarts. Each attempt sends a real prompt. A task rejected before acceptance retains its original message and attachments for retry.
Existing mobile clients receive ordinary message events instead of new resume-only event tags. The waiting notice shows the next attempt at the end of the conversation, updates there during retries, and disappears when waiting ends. No native mobile update is required for that notice. Web and desktop settings expose the opt-out only when the connected server supports it.
Why
Provider limits can interrupt work for hours. Keeping scheduling on the server lets unfinished sessions resume without leaving a client connected or checking availability manually. Increasing the retry interval limits traffic during long waits.
We have used iterations of this patch for roughly a week on multiple production clients by monkeypatching the official 0.0.37 and 0.0.38 releases. That use exposed issues with mobile compatibility, reset-time parsing, and scheduling, which informed these fixes. The latest corrections have automated coverage; they have not all been deployed or verified in production yet.
The scope is one feature: automatic resume and its status, settings, persistence, and provider handling. It reuses existing messages, settings, and event storage without adding dependencies. The branch is based on v0.0.38 and still needs forward-porting to current main. The cross-surface diff is larger than the contribution guidelines prefer.
UI Changes
Before: not applicable. The automatic waiting notice and setting did not exist in the official release.
After: the supplied mobile and desktop screenshots show the waiting notice in production. Their GitHub upload is pending because no browser connection is available. They show the earlier wording, before removal of the notice prefix and server-time suffix. An interaction video is not yet available.
The desktop error banner is dismissible. Repeated resumable failures do not add more error activities. Existing mobile binaries cannot gain native cancellation controls from a server patch; the environment opt-out is in web/desktop Settings.
Validation
Checklist
Implemented with Codex in T3 Code, with GPT-5.6 Luna implementation subagents and primary-agent review.