feat: add cli signup browser flow#13
Conversation
marckong
left a comment
marckong
left a comment
There was a problem hiding this comment.
Summary
Adds a volcano signup command that prefills the email from git config --global user.email, opens Volcano Web's /signup page (with the device user_code threaded through a next param), then completes the existing device-authorization handshake and persists credentials. Introduces a configurable VOLCANO_WEB_URL (compiled default + env override) and a make local target for pointing builds at a dev backend via .env.local.
WebSignupURLvalidation runs before a device code is allocated, so a misconfigured web URL fails fast.completeBrowserLoginis cleanly refactored to take an explicitbrowserURL, reused by both login and signup.- Good test coverage: URL builder, email prompt/validation, and the full signup browser flow.
Review
Clean, well-scoped change — build and the affected test packages (api, auth, cmd/auth, config, cmd/root) all pass, and the new code mirrors existing login conventions. No blocking issues found. Approving. 🚀
- Minor (non-blocking): at runtime, when the CLI is pointed at a loopback
VOLCANO_API_URLwithoutVOLCANO_WEB_URL,WebURL()still returns the production default (https://volcano.dev) — thelocalhost:3000fallback only exists inmake local. Documented, so likely intentional, but worth confirming it matches the intended local-dev UX.
Overall correctness: correct. No bugs that break existing code or tests; the patch is free of blocking issues.
swkeever
left a comment
swkeever
left a comment
There was a problem hiding this comment.
Reviewed signup flow, URL/config wiring, and build/test paths. No blocking issues found.
Signup previously built its browser URL from cfg.WebURL(), which fell back to the production default even when the CLI was pointed at a loopback or non-prod VOLCANO_API_URL without VOLCANO_WEB_URL set. That made signup open the wrong web origin while the device code lived on a different backend. Derive the signup origin (and device-approval path) from the device authorization response's verification URI instead — the same source login uses — so signup always follows the targeted backend. Precedence is now: explicit VOLCANO_WEB_URL override > backend-derived origin > compiled default. - add api.VerificationWebTarget to extract origin + device path from the device-auth response (prefers verification_uri_complete) - add config.WebURLOverride so an explicit env override wins without mistaking the compiled default for one - signup tests now assert backend-following behavior + override precedence - add VerificationWebTarget unit tests - docs: VOLCANO_WEB_URL is now an optional override Addresses review feedback on #13.
Fixed in 52a58cd by making |
Summary
volcano signupcommand with git email prefill/overrideVOLCANO_WEB_URLvia.env.localTests