REV-2277: Attach active team UID header at the GraphQL v2 chokepoint - #15326
Draft
warp-agent-staging[bot] wants to merge 2 commits into
Draft
REV-2277: Attach active team UID header at the GraphQL v2 chokepoint#15326warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
Adds a single chokepoint for attaching the X-Warp-Active-Team-UID header to outbound GraphQL v2 requests, so all GraphQL endpoints get the active team by default (REV-2277). - warp_server_client::base_client::ActiveTeamUid is a small shared cell consulted by BaseClient::graphql_request_options. Omitting the header (the default) is a no-op against current server behavior. - UserWorkspaces keeps the cell in sync with the focused window's current team assignment (reusing team_for_window, so a stale or unmapped assignment resolves to no active team), resyncing on window focus changes and on any team/workspace mutation. - This is provisional per the request's open question about what "active team" means for windowless GraphQL calls; the seam is intentionally the only place that would need to change if the answer becomes something else (e.g. a persisted active team).
Contributor
Author
|
This PR was generated with Warp. Comment |
Addresses review: the active-team header seam was never cleared at account boundaries, so a user who signed out of account A and into account B could send account B's GraphQL v2 requests carrying account A's team UID until UserWorkspaces resynced. - AuthManager::log_out now clears the seam before credentials are torn down. - AuthManager::on_user_fetched now clears it for fresh (non-refresh) logins/account switches, before any of the post-login GraphQL work (team/cloud-object polling, AI usage refresh, etc.) starts. - Added a regression test pinning log_out's clearing behavior. Also, per review: no test exercised the UserWorkspaces -> ActiveTeamUid wiring. Fixed sync_active_team_uid to read WindowManager's tracked active-window state (matching how the rest of the codebase treats 'the active window', and testable via overwrite_for_test) instead of the live platform query, which always returns None in the test platform backend. Added tests covering focus changes, team (re)assignment, and workspace-switch reconciliation. Testing the on_user_fetched success-path clearing end-to-end is not cheap: that path touches ~10 additional singletons (SettingsInitializer, UpdateManager, TeamTesterStatus, CloudPreferencesSyncer, LLMPreferences, PersistedWorkspace, GeneralSettings, SharedSessionManager, GlobalResourceHandlesProvider, PrivacySettings) that no existing test of this function currently registers. The clearing reuses the exact from_refresh signal that adjacent code in the same function already relies on for identical identity-vs-refresh semantics.
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
Adds the single chokepoint the client half of REV-2206 asked for: a place to attach the
X-Warp-Active-Team-UIDheader to outbound GraphQL v2 requests so all GraphQL endpoints get the active team by default.This PR is stacked on #15305 ("Send the active window's team UID on agent-mode requests") and branches off
factory/rev-2206-team-uid-header, reusinghttp_client::headers::WARP_ACTIVE_TEAM_UIDfrom that PR. The server counterpart is warp-server#15616 (REV-2274).Chokepoint:
BaseClient::graphql_request_optionsincrates/warp_server_client/src/base_client.rs— the same function every GraphQL v2 call already funnels through viasend_graphql_request/RequestOptions.headers, attached the same way as the existing ambient headers.Team source seam (provisional, awaiting requester confirmation):
warp_server_client::base_client::ActiveTeamUidis a smallArc<RwLock<Option<String>>>handle.graphql_request_optionsonly reads it (Noneomits the header entirely — a strict no-op against today's server).UserWorkspaces(app/src/workspaces/user_workspaces.rs) is the one place that writes to it, keeping it synced to the focused window's current team via the existingteam_for_window(which resolves toNonefor a window with no assignment or a stale assignmentreconcile_window_team_assignmentshasn't yet corrected — no invented fallback). It resyncs whenever the focused window changes (subscribing toWindowManager's tracked state) and whenever a window's team assignment or the workspace list changes.Account boundaries:
AuthManagerclears the seam directly (viaServerApiProvider::active_team_uid_handle) at every auth-session teardown (log_out) and at every fresh login/account switch (on_user_fetchedwhen!from_refresh), before any of that session's own GraphQL work starts. This prevents a stale team UID from a previous session/account from leaking onto a new one beforeUserWorkspaceshas a chance to resync.Open question: the requester (Tyler Lam) hasn't confirmed whether "active team" for windowless GraphQL calls should be the focused window's team (what this PR implements), a separately persisted active team, or omitted entirely. This PR is built so that answer only requires changing what calls
ActiveTeamUid::set— the header plumbing inBaseClientdoesn't change either way.Linked Issue
ready-to-spec/ready-to-implementchecklist doesn't apply here.Testing
crates/warp_server_client/src/base_client_tests.rs: the header is omitted by default, attached onceActiveTeamUid::setis called, cleared when set back toNone, and cannot be overridden viaAuthenticatedGraphqlConfig's static header configuration.app/src/workspaces/user_workspaces_tests.rsexercising theUserWorkspaces→ActiveTeamUidwiring: the active team follows the focused window's assignment (and only the focused window's), an unmapped or never-valid focused window omits the header rather than falling back, and switching the current workspace resyncs to the reconciled team.app/src/auth/auth_manager_tests.rspinning thatlog_outclears the active-team seam. Note: I did not add an end-to-end test throughAuthManager::on_user_fetched's success path (the fresh-login/account-switch clearing) — that path touches roughly 10 additional singletons (SettingsInitializer,UpdateManager,TeamTesterStatus,CloudPreferencesSyncer,LLMPreferences,PersistedWorkspace,GeneralSettings,SharedSessionManager,GlobalResourceHandlesProvider,PrivacySettings) that no existing test of this function currently registers, so it isn't cheap. The clearing reuses the exactfrom_refreshsignal that adjacent, already-relied-upon code in the same function uses for identical identity-vs-refresh semantics.cargo test -p warp_server_client --lib(50/50 passed),cargo test -p warp --lib workspaces::user_workspaces(56/56 passed), andcargo test -p warp --lib auth::auth_manager(10/10 passed).cargo clippy -p warp_server_client --all-targets --all-features -- -D warningsandcargo clippy -p warp --lib --tests -- -D warnings; both clean. Ran./script/format; no changes../script/runNot verified: I could not manually run the full desktop app in this sandbox (no display, and this repo's heavier
cargoinvocations have been OOM-killed in small environments before — a singlecargo check -p warp --libinvocation takes ~2 minutes and is close to what this environment can comfortably handle). I also could not run somewarp_server_clienttests under their default configuration: this sandbox's isolation-platform detection findsNSC_TOKEN_FILEset (it runs on Namespace infrastructure) but lacks thenscbinary, soambient_headers()fails before the code under test runs. I confirmed viagit stashthat this reproduces identically on the unmodified base branch, so it's a pre-existing sandbox limitation, not a regression; all tests pass withWARP_ISOLATION_PLATFORM=dockerset to work around it locally.Agent Mode