Pipe connected worker hosts into client host selectors#11226
Conversation
|
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 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 adds a connected self-hosted workers model backed by the public API and wires those worker host names into the orchestration config UI, run-agents confirmation card, and Cloud Mode host selector.
Concerns
⚠️ [IMPORTANT] The connected-worker cache is not cleared when authentication is unavailable, so stale host slugs can remain visible/selectable after auth loss.⚠️ [IMPORTANT] The orchestration host picker no longer preserves the local-channellocal-devoption for fresh selections.⚠️ [IMPORTANT] For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
Security
- Stale connected-worker host slugs can leak from one authenticated state into logged-out or next-user UI state unless the cache is cleared when auth is unavailable.
Verdict
Found: 0 critical, 3 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
| } | ||
|
|
||
| pub fn refresh(&self, ctx: &mut ModelContext<Self>) { | ||
| if !AuthStateProvider::as_ref(ctx).get().is_logged_in() { |
There was a problem hiding this comment.
workers and emit Changed when auth is unavailable so private host slugs are not shown after logout or auth loss.
| } else { | ||
| &["warp"] | ||
| }; | ||
| let mut hosts = ConnectedSelfHostedWorkersModel::as_ref(ctx_dropdown) |
There was a problem hiding this comment.
local-dev option. Local builds opening a fresh picker can no longer choose local-dev; preserve that local fallback when constructing the host list.
dmichelin
force-pushed
the
dan/remote-1644-known-workers-client
branch
3 times, most recently
from
5381c4d to
9a00284
Compare
| let mut connected_hosts = ConnectedSelfHostedWorkersModel::as_ref(ctx) | ||
| .worker_hosts_excluding(default_host.as_deref()) | ||
| .into_iter() | ||
| .filter(|host| !host.eq_ignore_ascii_case(ORCHESTRATION_WARP_WORKER_HOST)) |
There was a problem hiding this comment.
Should this be folded into worker_hosts_excluding?
| .into_iter() | ||
| .filter(|host| !host.eq_ignore_ascii_case(ORCHESTRATION_WARP_WORKER_HOST)) | ||
| .collect::<Vec<_>>(); | ||
| if ChannelState::channel() == Channel::Local { |
There was a problem hiding this comment.
Do we need this? The local dev host should get returned from the server as long as it's up and running
| hosts | ||
| } | ||
|
|
||
| pub fn refresh(&mut self, ctx: &mut ModelContext<Self>) { |
There was a problem hiding this comment.
What do you think about also refreshing when the Cloud Mode or Orchestration UIs are brought up? We can show the last fetched value while those refreshes are pending, and then still ensure the user can pick from all known hosts
| impl SingletonEntity for ConnectedSelfHostedWorkersModel {} | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
nit: these should go in a separate connected_self_hosted_workers_tests.rs file
| let mut connected_hosts = ConnectedSelfHostedWorkersModel::as_ref(ctx) | ||
| .worker_hosts_excluding(default_slug) | ||
| .into_iter() | ||
| .filter(|host| host != "warp") |
There was a problem hiding this comment.
Could this reuse the same constant as the orchestration picker?
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
dmichelin
force-pushed
the
dan/remote-1644-known-workers-client
branch
from
b2d7c5b to
9d07f07
Compare
2 participants
Description
This change adds connected worker hosts into the warp host selector chip. This query done every time the card pops up, so if a worker disconnects after startup or the hostname crashes the selector will not expose dead worker hosts.
Linked Issue
REMOTE-1644
Testing
cargo fmt --check --manifest-path /Users/dan/Development/warp/app/Cargo.tomlAlso loom
I have manually tested my changes locally with
./script/runLoom
https://www.loom.com/share/556238f3dee649ab929c5550aec7b2f5
Agent Mode
Co-Authored-By: Oz oz-agent@warp.dev