Only refresh tasks on RTC invalidation if a view is open.#11264
Only refresh tasks on RTC invalidation if a view is open.#11264liliwilson wants to merge 3 commits into
Conversation
liliwilson
force-pushed
the
lili/only-refresh-rtc-if-view-open
branch
from
74e141b to
c2ba08f
Compare
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR carries ambient task IDs through RTC update events, gates task-list refreshes on active list consumers, and adds a deferred catch-up refresh when a list view opens.
Concerns
- Force-refreshing an open task clears existing fetch state, which can spawn overlapping single-task requests for repeated RTC events and allow an older response to overwrite newer task data.
- Deferred updates while no view is open are collapsed to one timestamp and flushed through one capped list fetch, so high-volume updates can be missed when the view reopens.
- The PR description does not include manual testing details or screenshots/recording. Because this changes user-visible freshness for agent task details/list views, please attach evidence demonstrating the flow end to end.
- The checked-in tech spec links to a contributor-local
/Users/...file that reviewers and future readers cannot access.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| } | ||
| None => {} | ||
| } else { | ||
| self.task_fetch_state.remove(task_id); |
There was a problem hiding this comment.
InFlight guard when force-refreshing. RTC events for an open tab can arrive while the previous GET /agent/runs/{id} is still running; removing the fetch state here spawns overlapping fetches and an older response can overwrite newer task data. Only bypass cached task/failure cooldowns, not TaskFetchState::InFlight.
|
|
||
| // Flush dirty tasks accumulated while no view was open. | ||
| if let Some(dirty_since) = self.dirty_since.take() { | ||
| self.fetch_tasks_updated_after(dirty_since, ctx); |
There was a problem hiding this comment.
fetch_tasks_updated_after requests only INITIAL_TASK_AMOUNT tasks, so if more than 100 tasks changed while no view was open, reopening a list view drops the rest while RTC polling remains disabled. Track dirty task IDs or page until all updates are loaded before clearing dirty_since.
|
|
||
| ## Context | ||
|
|
||
| RTC invalidations for cloud agent tasks cause excessive `GET /api/v1/agent/runs` requests. During a bug bash with multiple concurrent agents on a team, this triggered 429 rate limiting that blocked spawning new agents. See [investigation report](/Users/liliwilson/Downloads/05_18_2025%20429s%20when%20starting%20cloud%20agents.md) for full context. |
There was a problem hiding this comment.
💡 [SUGGESTION] This link points to a local /Users/... path that other reviewers cannot open. Replace it with a repository-relative artifact, PR/issue link, or remove the link before merging.
1 participant
Description
This PR updates the client-side RTC logic to only refetch data for a task on an RTC invalidation if:
task_idfrom the RTC notification)Right now, whenever we get any RTC invalidation, we discard the task ID and do a refetch. The throttling introduced in #11236 limits this fetch to once per every 5s, but we really shouldn't be refetching on every RTC invalidation if we don't need to.
Linked Issue
ready-to-specorready-to-implement.Testing
./script/runScreenshots / Videos
Agent Mode