[multi-team P7a] Scope default host slug and agent attribution to the window's team - #15445
Open
warp-agent-staging[bot] wants to merge 5 commits into
Open
[multi-team P7a] Scope default host slug and agent attribution to the window's team#15445warp-agent-staging[bot] wants to merge 5 commits into
warp-agent-staging[bot] wants to merge 5 commits into
Conversation
Contributor
Author
|
This PR was generated with Warp. Comment |
… window's team Migrates two settings getters off the ambient `current_workspace().settings` read onto `TeamScope`: - `agent_attribution_setting_for_scope` replaces `get_agent_attribution_setting`, resolved fresh at render from the settings page's window. - `default_host_slug_for_scope` replaces the windowed uses of `default_host_slug`, resolved from the cloud-mode input's window and re-resolved when that window's team changes. - `any_team_has_default_host_slug` answers the windowless `/host` gate explicitly across every team, since that data source is shared with the TUI. `is_invite_link_enabled` and `is_discoverable` are untouched: they are genuinely workspace-only and have no `TeamSettings` equivalent.
…ange, cover the construction-time constraint - The host-selector resync applied `Some` and swallowed `None`, so a window moving to a team that configures no default host kept the previous team's slug in both the selector and the run config. `clear_default_host` mirrors `set_default_host`, and the view model's host is now cleared too. - The Warp Agent settings page only repainted on `TeamsChanged`; `window_team_uids` is not `Tracked`, so a window team change would not repaint the attribution toggle once a team switcher lands. - Added a test that resolves a scope inside a view's own build closure, asserting the `ViewContext` shape works there and a self-handle resolves nothing. Every other test resolves post-construction, where the difference is invisible.
warp-agent-staging
Bot
force-pushed
the
factory/multi-team-p7-host-slug-attribution
branch
from
August 22, 2026 19:56
bca4b2e to
721d9d7
Compare
…text - `effective_default_host` minted `TeamContextForOperation`, whose whole point is that it can be carried into an operation and recorded. Nothing was carried here: it was borrowed on the next line and dropped, so the type made a false claim that this host was pinned to the team it was chosen under, contradicting the doc comment three lines above it. - It now uses the borrowed `TeamContext`, resolved from the host selector's own handle rather than the `Input`'s. A view is absent from `view_to_window` until its own construction finishes, but a child it builds along the way is already registered, so the selector's handle resolves at build time where the Input's does not. Reading through the live mapping also means the host follows the selector if it is ever moved between windows. - Retargeted the construction-time test at that asymmetry: a self-handle resolves nothing mid-construction, a child handle resolves the window's team. - Narrowed `any_team_has_default_host_slug` to `pub(crate)`; its only caller is a default trait-method body inside `warp`. - Kept one test on the owned scope, redocumented as coverage that the getter behaves identically for both `TeamScope` implementors. Leaves the teamless guard in place pending the batched follow-up.
…ion caller Switching `effective_default_host` to the borrowed `TeamContext` removed the last production mint of `TeamContextForOperation`, so both the struct and `team_context_for_operation` became dead in the lib build and clippy failed on all four targets under -D warnings. Master carries these allows; my earlier rebase conflict resolution dropped them because at that point my code was the consumer that made them unnecessary. The comment now records why they are back, which is the useful signal: the pinning half of the contract has no production user yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Description
Part of the multi-team client scoping stack described by the tech spec on #15347, stacked conceptually after P0 (#15439, now merged into
master). This is slice P7a: it moves two small settings getters onto per-window team scope.Which workspace-settings fallback this retires. Both getters read
current_workspace().settingsdirectly today, which is not team-neutral data —GetEffectiveWorkspaceSettingsForWorkspaceresolves a single team server-side and falls through to a literalworkspaceTeamIDs[0]. This PR retires that read for:get_agent_attribution_settingis gone, replaced byagent_attribution_setting_for_scope(&impl TeamScope). Its only production caller, theAgentAttributionWidgeton the Warp Agent settings page, now resolves aTeamContextfrom its ownWeakViewHandleon every render.terminal/input.rsnow readsdefault_host_slug_for_scope, and the/hostslash-command gate now reads a new explicit cross-team accessor.A private
teamless_workspace_settings()provides the only remaining legitimate workspace fallback: the user belongs to no team at all. It applies the same empty-iterator guard asteams_allow_codebase_context.What is deliberately left out
ai::orchestration::resolve_default_host_slugstill reads workspace settings, now through a loudly-namedunscoped_default_host_slug()whose doc comment says so and forbids new callers. That function is the head of one connected component —resolve_recent_host_slug→host_snapshot→ the shared pickers inorchestration_controls.rs→ the plan card, the confirmation card,crates/warp_tui's orchestration block, andprepare_handoffwith eight sites inpipeline_tests.rs— which cannot move piecemeal and is comfortably over 1000 lines on its own.It also carries two decisions this slice cannot make:
register_windowis called only fromRootView::new(app/src/root_view.rs:1889), which is GUI-only, so no TUI code path has a registered window. The TUI orchestration block therefore needs an explicit cross-team accessor, and unlike a boolean, a host slug has no defensible aggregation direction to pick one when two teams disagree.worker_hostis baked intoAgentConfigSnapshotfor a cloud run being created — a chosen destination, so it wants aTeamContextForOperation, which is different discipline from the live picker defaults.Both are recorded on REV-2205 and tracked as P7b, which is deliberately parked rather than picked up. It should be re-scoped after the TUI
--teamflag lands: if the TUI can register a window and resolve a team, the windowless-value problem in (1) largely dissolves and the orchestration chain becomes mechanical scope-threading rather than a design question. Scoping it before then would be work done against the wrong shape.Explicitly not scoped
is_invite_link_enabledandis_discoverablesit in the same getter cluster and read the same way, but they are genuinely workspace-only (TeamSettingscarries orthogonalinvite_link/visibilityinstead). They are untouched.Deviations worth a reviewer's attention
effective_default_hostresolves through the host selector's own handle. It uses the borrowedTeamContext, not the ownedTeamContextForOperation: this read is live policy that expires with the read, and the owned type would claim the host was pinned to the team it was chosen under. The handle is the selector's rather than theInput's becausebuild_host_selectorruns insideInput::new, and a view is absent fromview_to_windowuntil its own construction returns — but a child it builds along the way is already registered (app.rs:3109inserts before the handle is returned at:3132). Reading through the live mapping also means the host follows the selector if it is ever moved between windows.None, it does not skip it. A window moving to a team that configures no default host now clears the selector's default (HostSelector::clear_default_host, the inverse ofset_default_host, deferring to a saved user selection the same way) and clears the view model'sworker_host. Leaving the previous value in place would point a cloud run launched from that window at the other team's self-hosted worker — the shape is pre-existing, butdefault_host_slug_for_scopereturning a deliberateNonein the cross-team case is what makes it bite.any_team_has_default_host_slug()is deliberately narrow. It answers only "does a default host exist anywhere", which is the availability question the/hostgate actually asks, and refuses to choose which slug. Known consequence: this widens the gate. A window on a team with no configured host now sees/hostoffered because some other team has one, whiledefault_host_slug_for_scopegives that window nothing. It is a widening rather than a leak — no other team's slug is exposed — and narrowing it needs a window inside a data source shared with the TUI, which is the P7b problem.What removing that mint revealed
Switching to the borrowed scope turned
TeamContextForOperationandteam_context_for_operationinto dead code and failed clippy on all four targets, because this PR's mint was the only production caller of the owned scope anywhere in the codebase. The#[allow(dead_code)]is restored on both, with comments naming warp#15443 as the incoming real caller — it moves one by value intoapply_onboarding_settingsfromroot_view.rs, which is what the owned type is for.This is worth reading as evidence rather than as a workaround. An architecture review had already concluded from reading code that the owned half of the scope design had one production mint and that the one mint was a misuse. Removing the misuse turned that into a compiler-checked fact: until #15443 lands, the pinning half of the contract has no production user at all, and the lint is what holds it alive. If there is an open question about whether the type earns its place, that is the strongest available input to it.
Adjacent, not fixed
Three things found while working here and deliberately left alone, recorded so they are not re-discovered:
set_default_hostand its caller disagree when a saved selection exists.HostSelector::set_default_hostdeliberately does not move the selection when the user has alast_selected_host, but the caller interminal/input.rsstill pushes the default into the view model'sworker_hostregardless. So the selector shows the saved host while the run config carries the default. This is pre-existing and orthogonal to team scoping, but it sits directly under the code this PR changes, so whoever picks up P7b will meet it —set_worker_hostis exactly what the handoff's pinned destination will be threading through.WarpAgentPageView::newinstalls two subscriptions toUserWorkspaces. The filtered one at:625(widened here) and an unfiltered one at:784that callsctx.notify()on every event. The second means the page already repaints onWindowTeamChangedtoday, so the widening is belt-and-braces rather than a behaviour fix — but relying on a redundant unfiltered subscription for correctness is fragile, and the duplicate should be collapsed by someone in a change that is only about that.is_shared_block_title_generation_toggleablereads billing entitlement per window. Atapp/src/settings_view/warp_agent_page.rs:2929it does a team-scoped read ofteam.billing_metadata.customer_typeto decide Enterprise-ness. That predates the entitlement-versus-policy line drawn on REV-2205 — billing entitlement should stay workspace-level — andTeam::from_gql(gql_convert.rs:1339-1361) clones workspacebilling_metadataverbatim onto every team, so the team-scoped read returns workspace data anyway. It belongs to #15372; flagging rather than touching it.Linked Issue
REV-2205 — [multi team] Scope team settings on the client to window context
ready-to-specorready-to-implement.Testing
This change was not compiled, linted, or tested locally. The sandbox this was written in has ~4 GB of RAM and
rustcis SIGKILLed part-way through thewarpcrate even at-j 1with debuginfo and incremental disabled../script/formatwas run and passes;cargo clippyand the test suite could not be run at all. CI is the primary verification here, which is why this was opened ready for review rather than as a draft — and CI came back green on the first commit (formatting + clippy on Linux, macOS, Windows and wasm; tests on all four; every release-flag compile).Thirteen unit tests were added to
app/src/workspaces/user_workspaces_tests.rs, one was rewritten (test_agent_attribution_default_with_no_workspace, which now resolves through a scope), and three were removed (test_agent_attribution_forced_on_by_team/_forced_off_by_team/_respects_user_setting) because they asserted the workspace-settings read this PR retires and their scoped equivalents replace them. Between them they cover:Two windows on teams with opposing policies each resolve their own team, for attribution and for the host slug, via both the
WeakViewHandleshape the settings page uses and theViewContextshape the host selector uses.A window reconciled onto a new team reports the new team's policy on the next read — the stale-modal guard, and the reason neither of these reads is captured.
A window with no team gets neither another team's setting nor the workspace's.
A genuinely teamless user still reads workspace settings, for both getters.
any_team_has_default_host_slugis true when one of two teams configures a host, false when neither does even though workspace settings do, and true for a teamless user with a workspace-level host.A view resolving a scope inside its own
add_typed_action_viewbuild closure gets its team through theViewContextand gets nothing through a handle to itself. This is the case that matters: every other test here resolves post-construction, where a handle would have worked equally well, so without it the exact mistake described above passes the whole suite.No pinning test, because nothing in this slice pins.
Uncovered behaviour, stated plainly: there is no test that a team change actually schedules a repaint of the attribution toggle or re-pushes the host into the selector.
take_all_invalidations_for_windowandsimulate_render_framearepub(crate)insidewarpui_core, so nothing inwarp's tests can assert a render happened; the tests prove the data resolves per-team, not that the UI redraws. Every Group 1 PR in this stack has the same gap.They did run, despite not running locally. Confirmed from the
Run Linux testsjob log rather than inferred from a green tick: all fourteen tests in this cluster appear by name, insideSummary [ 57.781s] 11056 tests run: 11056 passed, 34 skipped. So "could not be run locally" does not mean "unverified" here — it means the verification happened in CI instead of on my machine../script/runScreenshots / Videos
None. Demonstrating either change needs a logged-in multi-team workspace with opposing attribution policy or different
default_host_slugvalues on two teams, which this environment cannot set up.Agent Mode