fix(ui,cli): count down provider retry wait from the event timestamp - #3400
fix(ui,cli): count down provider retry wait from the event timestamp#3400me2seeks wants to merge 1 commit into
Conversation
…pache#3393) A long provider-mandated Retry-After (e.g. a subscription quota window reset of ~4.5h from kimi-k3 / OpenCode Go) made the retry indicator pin the original delay for the whole sleep: both the TUI activity strip and the desktop banner rendered the static delayMs snapshot while the 1s ticker re-rendered everything around them. Render the remaining wait as delayMs - (now - retry.ts) instead: - TUI strip reuses the shared duration formatter, so hours-long waits read '4h 28m 3s' rather than a raw five-digit second count - desktop banner follows the elapsed-clock determinism contract: first paint and frozen fixtures show the provider's delay, live mounts tick the countdown once per second The wait itself is unchanged; whether to cap extreme Retry-After values in the runtime retry loop is a separate policy question.
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
Automated review of exact head 5eb038d3ddb75cb0d8eea8a614ac2c6da8a407a1.
The current main UI and TUI both render the original delayMs, so the bug still exists. The implementation correctly derives remaining time from the event timestamp, clamps at zero, preserves the frozen-fixture contract, and cleans up the one-second interval. The focused UI build and new UI tests passed locally (2/2). I found no P0-P2 product defect.
Non-blocking test-quality gap: provider-retry-countdown.test.tsx proves the timestamp calculation by rerendering with an older event, but it never advances the mounted interval. The test would still pass if the interval were deleted. Please consider advancing fake timers on one mounted banner so the live ticking behavior is pinned directly.
Merge readiness: not ready yet. This exact head has no hosted checks and still requires independent human review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for fixing the frozen retry presentation without moving retry authority out of Runtime. I reviewed exact head 5eb038d3ddb75cb0d8eea8a614ac2c6da8a407a1. Reusing the TUI heartbeat and keeping the Desktop timer local to the banner are both appropriately scoped. I left two P2 presentation-contract issues below.
AI-assisted review disclosure: Codex performed the exact-head analysis, and an independent reviewer agent adversarially checked clock semantics, remote Runtime Host paths, timer lifecycle, accessibility, and regression coverage. I verified the source paths, live state, and final severity.
| ? 0 | ||
| : nowMs === undefined | ||
| ? retry.delayMs | ||
| : Math.max(0, retry.delayMs - (nowMs - retry.ts)); |
There was a problem hiding this comment.
Thanks for deriving the remaining delay from the event rather than storing another countdown state. [P2] retry.ts is produced by the Runtime Host clock, while nowMs comes from the Client clock. On the supported remote-Host path, ordinary clock skew can therefore make a multi-hour retry display 0s immediately or show substantially more time than the Host will actually wait. Could the Host projection provide an authoritative remaining value, or otherwise establish a single clock domain before the Client starts ticking it down? The same calculation in the TUI has the same boundary.
| useEffect(() => { | ||
| if (retry.phase !== 'scheduled' || !isTimeDrivenMotionEnabled(rootRef.current)) return; | ||
| setNowMs(Date.now()); | ||
| const tick = window.setInterval(() => setNowMs(Date.now()), ELAPSED_TICK_MS); |
There was a problem hiding this comment.
Thanks for keeping the countdown visually live. [P2] Because this banner is a role="status" live region, changing its title every second can cause screen readers to announce every tick—potentially for hours during a quota wait. Could the changing visual countdown be hidden from the live region while exposing a stable accessible status, following the pattern already used by the running-turn indicator? A focused accessibility regression would help keep the timer from reintroducing this.
3 participants
Fixes #3393.
Problem
When a provider returns a long
Retry-After(e.g. a subscription quota window reset — kimi-k3 / OpenCode Go returns ~4.5h when its 5h quota is exhausted), the retry indicator pins the original delay for the entire sleep:Retrying in 16083s (2/10)never counts down, indistinguishable from a hung process. The TUI's 1s heartbeat re-renders the strip and the siblingWorking… <elapsed>counter ticks, but the retry line renders a staticdelayMssnapshot.Fix
Render the remaining wait as
delayMs - (now - retry.ts)in both clients:pi-transcript.ts): reuses the shared duration formatter, so hours-long waits read4h 28m 3sinstead of a raw five-digit second count; floors at0s.chat-turn.tsx): follows the same determinism contract as the turn's elapsed clock — first paint and frozen fixtures show the provider's original delay; live mounts tick the countdown once per second.Scope
Presentation layer only: the wait itself is unchanged. Honoring a multi-hour
Retry-Afterfrom a subscription quota window is intentional (the turn resumes when quota resets); whether the runtime should additionally cap extreme values is a separate policy question noted in the issue.Tests