feat: retry Cloud Mode initial runs after GitHub auth#10973
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 |
There was a problem hiding this comment.
Overview
This PR centralizes GitHub auth redirect URL construction and adds Cloud Mode retry handling after GitHub auth completes.
Concerns
- The added ambient-agent model test helper constructs
SpawnAgentRequestwithout the requiredsnapshot_disabledfield, so the test target will not compile against the current request struct.
Verdict
Found: 0 critical, 1 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
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
ef891b5 to
7c1af4a
Compare
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review 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 |
There was a problem hiding this comment.
Overview
This PR centralizes GitHub auth redirect URL construction and adds a Cloud Mode initial-run retry after GitHub auth completes.
Concerns
- The auth-completed event is broadcast through the singleton notifier to every
AmbientAgentViewModel, so one OAuth callback can retry multiple panes that are waiting for GitHub auth instead of only the original/focused pane.
Verdict
Found: 0 critical, 1 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
| me.validate_selected_harness(ctx); | ||
| }); | ||
|
|
||
| ctx.subscribe_to_model(&GitHubAuthNotifier::handle(ctx), |me, event, ctx| { |
There was a problem hiding this comment.
NeedsGithubAuth with a cached initial request. Scope the retry to the pane/conversation selected by the deeplink, or carry an auth attempt id through the callback before spawning.
There was a problem hiding this comment.
This is a good point. I dunno how likely it is that we'll actually run into a situation where a user has two cloud mode panes pending and executes an auth refresh in one. In that case, it might be surprising for the retry to occur in each pane. It does increase the size of the PR. Curious to hear what folks things about relative value vs. likelihood for this case.
There was a problem hiding this comment.
It seems pretty unlikely, and even if it were to happen I'm not sure its that undesirable. I'm assuming this is the easier approach to take? if so Im fine with it
There was a problem hiding this comment.
Yeah, I thought about it a bit more and I think it might actually be nicer if the retry did trigger on every pane so you didn't have to go and figure out how to recover in every session.
| request: Option<SpawnAgentRequest>, | ||
|
|
||
| /// Request needed to retry an initial run after GitHub auth completes. | ||
| initial_run_retry_request: Option<SpawnAgentRequest>, |
There was a problem hiding this comment.
does this actually need to be stored separately from request?
There was a problem hiding this comment.
Strictly, no. I thought it would be helpful to separate out the request we retry from the one we persist. But how about I continue a cloud session and change this 😄
zachbai
left a comment
There was a problem hiding this comment.
that was quick - thanks for taking this!
Remove the duplicate initial-run retry request field and rely on the existing stored spawn request when retrying after GitHub auth completes. Co-Authored-By: Oz <oz-agent@warp.dev>
Description
Implements APP-4458: when an initial Cloud Mode spawn requires GitHub auth, the auth URL now deep-links back into Warp, focuses the Cloud Mode pane, and retries the original initial-run request after auth completes.
This also centralizes GitHub auth redirect URL construction so the environment form and Cloud Mode setup flow share the same redirect-target/source handling.
Linked Issue
Linear: https://linear.app/warpdotdev/issue/APP-4458/deep-link-back-into-warp-after-authenticating-github-from-initial
ready-to-specorready-to-implement.Testing
cargo fmtcargo test -p warp terminal::view::ambient_agent::model::tests:: --libcargo test -p warp test_build_auth_url_with_next --libcargo fmt --check./script/runScreenshots / Videos
Demo video: https://www.loom.com/share/ac33da8156eb4949a14575c95209b6b9
Agent Mode
Co-Authored-By: Oz oz-agent@warp.dev