Skip to content

Scope generate_code_review_content to the window's team - #15560

Open
warp-agent-staging[bot] wants to merge 4 commits into
masterfrom
factory/team-header-code-review-content
Open

Scope generate_code_review_content to the window's team#15560
warp-agent-staging[bot] wants to merge 4 commits into
masterfrom
factory/team-header-code-review-content

Conversation

@warp-agent-staging

Copy link
Copy Markdown
Contributor

Description

Scopes the client-side POST /ai/generate_code_review_content call (commit-message autogen and PR title/body generation in GitDialog) to the window's team by sending X-Warp-Team-Uid.

Previously this request carried no team header. The server falls through GetActiveTeamOrFallbackToPrincipalGetTeamForPrincipal, which picks the caller's lowest team id — an arbitrary team, not the one the window is actually working in. Since this endpoint's server handler (generate_code_review_content.go) writes billing metadata, an unscoped call attributed usage to the wrong team for any multi-team user whose window wasn't on their lowest-id team.

The team now comes from the window (ViewContext<GitDialog>UserWorkspaces::team_context_for_view), resolved right before the model call so it can't drift to a fallback. That scope is threaded as a RequestTeamScope through DiffStateModelLocalDiffStateModelgit_actions::{run_commit_chain, create_pr, generate_commit_message}AIClient::generate_code_review_content, which is the one call site that actually sends the header (mirrors the /ai/multi-agent pattern from #15355).

The remote-server daemon path (RemoteDiffStateModel → RPC → ServerModel handlers) also calls this same endpoint, but the daemon is headless and has no window/team concept at all, so there's no live scope to resolve there. Those call sites now pass RequestTeamScope::none_for_headless_context(), explicitly, rather than guessing a team — this is unchanged behavior for that path (no header was sent before either).

Linked Issue

N/A — this PR was dispatched directly as part of a header-scoping cleanup pass, not from a tracked issue.

Testing

  • ./script/format and cargo clippy -p warp --lib --tests -- -D warnings pass with no findings.
  • cargo nextest run -p warp for code_review::, remote_server::, and server_api::ai:: (150 tests, 149 passed / 128 + 149 across both runs; the one failure, ambient_agent_headers_for_task_overrides_existing_cloud_agent_header, is a pre-existing sandbox limitation — it also fails on unmodified master here because the nsc namespace command is denied in this environment — and is unrelated to this change).
  • No behavior test was added: this is a plumbing change (adding a header value) with no new branching logic to regress-test, and the endpoint's request/response shape is unchanged.
  • Not manually verified end-to-end against a live multi-team account (no GUI/browser available in this environment); the header value itself is exercised by existing coverage of RequestTeamScope/TEAM_UID_HEADER from [multi-team] Scope multi-agent requests to a team #15355.

Screenshots / Videos

Not applicable — no UI change.

Agent Mode

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

Adds the X-Warp-Team-Uid header to the client-side POST /ai/generate_code_review_content call so commit-message and PR title/body generation resolve against the window's team instead of the server's lowest-team-id fallback.
All three callers are in-crate (remote_server::server_model); the constructor doesn't need to be more public than team_uid(), which it feeds.
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

This PR was generated with Warp.

Comment @warp-factory on this PR to send it follow-up work.

View run View conversation

Drop the comments that restate which code path a parameter reaches or the resolve-before-call sequencing; a reader following the code already learns that. Keep the daemon-headless comments and the trimmed doc on none_for_headless_context, which document a deliberate decision rather than mechanism.
Its only callers are the remote-server daemon's RPC handlers, which are only reachable from a #[cfg(not(target_family = "wasm"))] worker command. On the wasm target those handlers are themselves dead code, so this new pub(crate) fn had no live caller there, tripping the dead_code lint under -D warnings. Same idiom as list_agent_messages_for_task a few lines above in ai.rs.
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.

0 participants