[login] support hosted success redirects - #28745
Conversation
6cef808 to
e90b376
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e90b376501
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let request = ClientRequest::LoginAccount { | ||
| request_id: RequestId::Integer(3), | ||
| params: v2::LoginAccountParams::Chatgpt { | ||
| app_brand: None, |
There was a problem hiding this comment.
does app_brand: None only happens when use_hosted_login_success_page is false?
There was a problem hiding this comment.
The hosted branch still defaults an omitted brand to Codex; device-code login explicitly uses the local LoginSuccessPage default. The shared helper now takes both values explicitly in 944c74e.
…ted-login-success
|
/merge |
What
Adds an optional hosted login-success redirect path for app-server login requests.
appBrandvalue and forwards it to the hosted page asapp_brandso web can select the correct asset.Why
This supports the hosted Codex login success page rollout without changing existing login behavior by default. The Codex Apps frontend can gate the opt-in with Statsig after the hosted web page.
Rollout safety
Validation
just fmtjust fix -p codex-login -p codex-app-server-protocol -p codex-app-server -p codex-app-server-test-client -p codex-tuijust test -p codex-loginjust test -p codex-app-server-protocoljust write-app-server-schemagit diff HEAD --checkThe focused login and protocol run passed all 380 tests.
I also started the broader
just test -p codex-app-server; it compiled successfully, then many tests failed on this machine because spawned test servers tried to use the ambient/Users/rafaelj/.codex/sqlitestate DB, which is read-only in this sandbox. I stopped that run after confirming the failures shared that environment issue.