Only refresh tasks on RTC invalidation if a view is open. by liliwilson · Pull Request #11264 · warpdotdev/warp

liliwilson · 2026-05-19T07:07:30Z

Description

This PR updates the client-side RTC logic to only refetch data for a task on an RTC invalidation if:

The task is actively being viewed in a given task (using the task_id from the RTC notification)
We have the agent management view or conversation sidebar open

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.

Testing

I have manually tested my changes locally with ./script/run

Tested locally, loom: https://www.loom.com/share/c4b23aaab2c4447c8839631941bbcce2

Agent Mode

Warp Agent Mode - This PR was created via Warp's AI Agent Mode

liliwilson · 2026-05-19T20:24:16Z

/oz-review

oz-for-oss · 2026-05-19T20:24:30Z

@liliwilson

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

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

oz-for-oss · 2026-05-19T20:28:33Z

+
+        // 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);


⚠️ [IMPORTANT] This catch-up can miss updates: 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.

I think this is fine, since the management views already behave this way—we fetch more things when you apply a filter, but some tasks are genuinely inaccessible via this view because they are too old

Right, the view should always be showing the most recent 100 tasks that match the filters - we're not trying to show all tasks

oz-for-oss · 2026-05-21T06:02:13Z

@liliwilson

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR threads RTC task IDs through UpdateManager and changes AgentConversationsModel to avoid broad task-list refreshes unless a list surface is open, with dirty-state flushing when a list view reopens.

Concerns

The throttled list refresh timer can still fire after all list consumers close, so pending RTC events recorded while a list view was open can still trigger a broad list fetch with no view open.

Verdict

Found: 0 critical, 1 important, 0 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

oz-for-oss · 2026-05-21T06:05:26Z

+        match std::mem::take(&mut self.rtc_task_refresh_throttle_state) {
+            RtcTaskRefreshThrottleState::Idle => {
+                self.fetch_tasks_updated_after(timestamp, ctx);
+                self.start_rtc_task_refresh_throttle_timer(ctx);


⚠️ [IMPORTANT] This timer can outlive the list consumers: if a pending timestamp is recorded while a list view is open and the user closes all list views before the cooldown fires, start_rtc_task_refresh_throttle_timer will still run fetch_tasks_updated_after, causing the broad refresh this PR is trying to avoid. Abort the RTC throttle when the last list consumer closes or re-check has_list_consumers before flushing the pending timestamp.

This seems fine? A few unnecessary refreshes if someone happens to toggle the management view shouldn't add that much traffic

bnavetta · 2026-05-21T09:35:17Z

+        match std::mem::take(&mut self.rtc_task_refresh_throttle_state) {
+            RtcTaskRefreshThrottleState::Idle => {
+                self.fetch_tasks_updated_after(timestamp, ctx);
+                self.start_rtc_task_refresh_throttle_timer(ctx);


This seems fine? A few unnecessary refreshes if someone happens to toggle the management view shouldn't add that much traffic

bnavetta · 2026-05-21T09:36:54Z

+
+        // 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);


Right, the view should always be showing the most recent 100 tasks that match the filters - we're not trying to show all tasks

bnavetta · 2026-05-21T09:39:11Z

        self.abort_rtc_task_refresh_throttle();
        self.active_data_consumers_per_window.clear();
        self.task_fetch_state.clear();
+        self.dirty_since = None;


Should fetch_tasks_updated_after also clear dirty_since? Once we do a list refresh, either:

The updated but unfetched task(s) will be included in the list response

They'll be old enough to fall out of the view

bnavetta · 2026-05-21T09:40:01Z

    ) {
-        ctx.emit(UpdateManagerEvent::AmbientTaskUpdated { timestamp });
+        let Ok(task_id) = task_id.parse::<AmbientAgentTaskId>() else {
+            log::warn!("Ignoring AmbientTaskUpdated with unparseable task_id: {task_id}");


Probably worth using log::error or report_error! - if the server is sending back task IDs that the client can't parse, we'd need to fix it

cla-bot Bot added the cla-signed label May 19, 2026

Only refresh tasks on RTC invalidation if a view is open.

c2ba08f

liliwilson force-pushed the lili/only-refresh-rtc-if-view-open branch from 74e141b to c2ba08f Compare May 19, 2026 18:43

Updates

423082a

oz-for-oss Bot reviewed May 19, 2026

View reviewed changes

Update force_refresh to respect the task fetch state map

3ea7c51

liliwilson force-pushed the lili/only-refresh-rtc-if-view-open branch from 6820a1e to dd4ef6d Compare May 21, 2026 06:00

More cleaning up

a01931c

liliwilson force-pushed the lili/only-refresh-rtc-if-view-open branch from dd4ef6d to a01931c Compare May 21, 2026 06:01

liliwilson requested a review from bnavetta May 21, 2026 06:01

liliwilson marked this pull request as ready for review May 21, 2026 06:02

oz-for-oss Bot reviewed May 21, 2026

View reviewed changes

bnavetta approved these changes May 21, 2026

View reviewed changes

Conversation

liliwilson commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Agent Mode

Uh oh!

liliwilson commented May 19, 2026

Uh oh!

oz-for-oss Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

Uh oh!

oz-for-oss Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

liliwilson May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oz-for-oss Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

bnavetta May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

liliwilson commented May 19, 2026 •

edited

Loading

oz-for-oss Bot commented May 19, 2026 •

edited

Loading

oz-for-oss Bot commented May 21, 2026 •

edited

Loading