Skip to content

Surface team member removal failures in Teams settings UI - #14964

Open
warp-agent-staging[bot] wants to merge 1 commit into
masterfrom
factory/remove-team-member-error-feedback
Open

Surface team member removal failures in Teams settings UI#14964
warp-agent-staging[bot] wants to merge 1 commit into
masterfrom
factory/remove-team-member-error-feedback

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

When removing a team member from Settings > Teams fails on the server, the client showed no feedback at all — the member just stayed in the list, and the only trace was in the log file.

Root cause: UserWorkspaces::remove_user_from_team spawned its GraphQL request with the shared on_workspaces_updated completion callback. That callback's error arm only calls report_error!(e.context("Failed to load user workspaces")) — it never emits an event, so the Teams settings UI had nothing to react to, and the logged context string was misleading (it's a member-removal failure, not a workspaces-load failure).

Fix: follow the existing pattern used by every other team mutation in user_workspaces.rs (e.g. on_add_invite_link_domain_restrictions, on_team_member_role_set). Added a dedicated on_remove_user_from_team handler that emits RemoveUserFromTeamRejected(err) / RemoveUserFromTeamSuccess on UserWorkspacesEvent, and wired it in as the spawn callback for remove_user_from_team instead of the shared on_workspaces_updated. TeamsPageView::handle_model_event now surfaces the rejected event as an error toast that includes the server's actual error message, and refreshes the member list on success exactly as before.

This only changes the removal path — on_workspaces_updated's shared error arm (used by every other flow via on_workspaces_updated(Ok(...)) from their own success arms) is untouched.

Linked Issue

Fixes APP-5317.

Testing

  • Added test_remove_user_from_team_rejected_emits_error_event_without_updating_workspaces and test_remove_user_from_team_success_emits_success_event_and_refreshes_members in app/src/workspaces/user_workspaces_tests.rs, covering both the rejected path (event emitted with the server's error message, local state untouched) and the success path (event emitted, member list refreshed).
  • cargo nextest run -p warp -E 'test(user_workspaces) or test(teams_page)' — 54 passed.
  • cargo fmt -- --check — clean.
  • cargo clippy -p warp --tests -- -D warnings — clean. (Note: cargo clippy -p warp --all-targets --all-features currently fails to compile due to pre-existing clippy errors in warp_completer's v2 feature, unrelated to this change, reproducible on master before this PR, and consistent with presubmit's own exclusion of that crate from --all-features.)
  • Full CI on this PR (Warp CI matrix: Linux/macOS/Windows tests, formatting + clippy for Linux/macOS/Windows/wasm, release-flag compilation variants, misc checks) — all green.
  • Visual verification attempted, not achieved — reporting concretely instead of asserting it's unnecessary. I tried to capture a screenshot of the real on-screen error toast: built the branch cleanly, applied a local (uncommitted, reverted) hack to force RemoveUserFromTeam to fail with the exact error string from the original bug report and to inject a removable team member, then launched the real Warp GUI under computer use. The app authenticated as an Oz agent service account rather than a real Warp user (Unauthorized: Expected a user account), so it landed on the logged-out onboarding screen and never reached an authenticated terminal, let alone Settings > Teams. No user-scoped credential (STAGING_USER_WARP_API_KEY) was available in that sandbox either. So the toast render path is exercised only by code review + the fact that the message string is built and passed through unchanged (format!("Failed to remove team member: {err}")) to the pre-existing show_toast/ToastFlavor::Error component already used by sibling rejection flows (e.g. SetTeamMemberRoleRejected) elsewhere in this same file. A screenshot from an environment with a real user-scoped Warp credential and an existing multi-member team would close this gap; happy to produce one if such credentials/account can be provided.

Agent Mode

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

CHANGELOG-BUG-FIX: Fixed an issue where removing a team member from Settings > Teams would fail silently with no error shown if the removal was rejected by the server.

Conversation: https://staging.warp.dev/conversation/00f772cb-d726-478e-a64f-a77a139df241
Run: https://oz.staging.warp.dev/runs/019ff2ae-45b1-78f2-9900-e778550deff4

This PR was generated with Oz.

Previously, remove_user_from_team spawned with
on_workspaces_updated as its completion callback, so a failed
removal only logged "Failed to load user workspaces" and never
told the Teams settings UI anything went wrong. The member just
stayed in the list with no visible feedback.

Add a dedicated on_remove_user_from_team handler (mirroring the
pattern used by every other team mutation) that emits
RemoveUserFromTeamRejected/RemoveUserFromTeamSuccess events, and
wire it into remove_user_from_team instead of the shared
on_workspaces_updated error arm. Surface the rejected event as an
error toast in TeamsPageView that includes the server's error
message, and refresh the member list on success as before.

Fixes APP-5317.

Co-Authored-By: Warp Agent <agent@warp.dev>
@warp-agent-staging
warp-agent-staging Bot requested a review from jefflloyd August 11, 2026 21:47
@cla-bot cla-bot Bot added the cla-signed label Aug 11, 2026
@warp-agent-staging
warp-agent-staging Bot marked this pull request as ready for review August 11, 2026 22:32

@warp-agent-staging warp-agent-staging Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

Adds a dedicated on_remove_user_from_team handler with RemoveUserFromTeamSuccess/RemoveUserFromTeamRejected events and surfaces the failure as a toast on the Teams page, so a rejected removal is no longer silent. Reviewed and found no correctness defects; two items below need your judgment rather than a code change.

Concerns

  • Toast copy for the failure you actually hit. For the reported error, the toast renders as Failed to remove team member: missing response data for RemoveUserFromTeam: Not found: no rows in result set — intact and under the truncation threshold, but "missing response data" is client-internal vocabulary and "no rows in result set" is a database-layer leak, so you learn that the removal failed and still not why. There are two error shapes here: a server UserFacingError yields a clean server-authored sentence, while a top-level GraphQL error yields this wrapper, and your case is the second. Your call between keeping it as-is, using static copy with the error appended only when it is user-facing, or appending always with a generic tail such as "please contact support"; see the inline comment.
  • Visual proof is missing, and unblocking it needs a credential. The Teams page has no view-event harness, so the five lines that actually put the message on screen are covered by no test, and an attempt to capture the real toast failed: the client authenticated as an agent service account and never got past the logged-out screen (Unauthorized: Expected a user account). A user-scoped staging or dogfood credential for an account on a team with at least one other member is enough to finish this, and the capture would also settle the copy question above at a glance.
  • Successful removals gained a confirmation toast ("Removed team member") that the report did not ask for. It is consistent with the sibling flows on this page, and the member list still refreshes exactly as before; flagging it only so the addition is deliberate rather than discovered.

Verdict

Checks: build pass, tests pass, CI green, visual proof missing

Found: 0 critical, 2 important, 1 suggestion, 0 nits

}
UserWorkspacesEvent::RemoveUserFromTeamRejected(err) => {
self.show_error(
format!("Failed to remove team member: {err}"),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is passed through without a .context() wrapper, which is deliberate and correct — anyhow's Display shows only the outermost message, so wrapping would have hidden the server's text. The open question is what that text reads like for the top-level GraphQL error shape you hit: Failed to remove team member: missing response data for RemoveUserFromTeam: Not found: no rows in result set.

Note this is the only toast on this page that embeds the error text; every sibling uses static copy ("Failed to update team member role", "Error leaving team"). The divergence is what satisfies your acceptance criterion, so it is defensible — but if you would rather not surface internal plumbing to operators, the alternative is a static headline plus the server message only when it is a UserFacingError. Tell me which you want and I will turn it around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant