feat: [AI-7520] Altimate LLM Gateway CLI signup + onboarding UX#1001
feat: [AI-7520] Altimate LLM Gateway CLI signup + onboarding UX#1001saravmajestic wants to merge 5 commits into
Conversation
- Add an `oauth` `method:"auto"` to the Altimate auth plugin: bind a loopback server on `localhost:7317`, open the browser to the web authorize page, verify `state`, and save the gateway credential to `~/.altimate/altimate.json` - Surface `altimate-backend` first in provider selection (TUI + clack) with the "Recommended · best tool-calling · 10M free tokens" hint Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uth success
- Add `WelcomePanel` boot box (readiness-aware tips + "What is Altimate Code")
on the home screen; first run is a welcoming panel, not an auto-opened modal
- Restructure the model picker into READY / NEEDS-SETUP; add the curated
`DialogModelWelcome` (top 5 providers + "Search all providers"), Big Pickle
fallback, and `useReady` / `markSetupComplete`
- `/connect` opens the curated picker
- Altimate LLM Gateway sign-in confirms inline ("Authentication successful") and
auto-closes (auto-selecting a model) instead of dropping into the model picker
- Don't force Google: land on the sign-up page and let the user choose
(drop `google_start`); reword the sign-in instruction
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
- /auth: sign in to the Altimate LLM Gateway directly via the OAuth loopback, skipping the provider picker (new `DialogAltimateAuth`) - /logout: clear the stored gateway credential (`AltimateApi.clearCredentials`) and disconnect (dispose + bootstrap; the provider loader drops the now-stale auth-store entry on reload) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DialogModelWelcome now tags each row with its providerID (and modelID for Big Pickle) and compares against local.model.current(), rendering a bright-green ✓ plus a "· selected" note on the active provider — so the user can see at a glance that e.g. Altimate LLM Gateway is already selected. Bright green (diffHighlightAdded) is used because plain ANSI green renders dim in some terminals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ebase safety) Shrink our footprint on the upstream opencode `dialog-model.tsx` so future upstream merges are easier to rebase: - Move `useReady`/`markSetupComplete`, `DialogModelWelcome`, and `DialogBigPickleConfirm` into a new altimate-owned `component/altimate-onboarding.tsx` (zero rebase surface — our file) - `dialog-model.tsx` (424 → 160 lines) now holds only pristine `useConnected` plus the `DialogModel` READY/NEEDS-SETUP restructure, fully wrapped in `altimate_change` markers so an upstream conflict is confined and human-resolvable - Repoint imports in `app.tsx`, `home.tsx`, `dialog-provider.tsx`, `welcome-panel.tsx` The onboarding file imports `DialogModel`/`useConnected` back from `dialog-model`, but only inside callbacks/JSX — the circular reference is runtime-only and safe. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Consensus code review — PR #1001
Independent reviews from 6 models (Claude, GPT 5.4 Codex, Kimi K2.5, MiniMax M2.7, Qwen 3.6, MiMo V2.5 Pro), converged and then re-validated line-by-line against the checked-out code.
Verdict: changes requested. The onboarding refactor and OAuth loopback design are well-structured (crypto-random state, CSRF state check, 0o600 credential file, cleanly isolated onboarding). But the new loopback auth server has correctness/reliability bugs and localhost security-hardening gaps — 7 major, 9 minor — all in the auth path. See inline comments.
Nits (not inlined): recent-models section dropped from the model picker; Big Pickle won't show a ✓ in the full picker after selection; error page doesn't window.close() like the success page; documented circular import between altimate-onboarding.tsx and dialog-model.tsx; silent open() failure (mitigated — the URL is still rendered); no ALTIMATE_CALLBACK_PORT override.
Positives: crypto-random state + CSRF check, 0o600 credentials, clean rebase-safe isolation of onboarding, reuse of existing provider onSelect handlers, reliable server teardown on the normal path.
Missing tests: the entire OAuth loopback flow (early callback, invalid/missing state, error branch, timeout, port collision, persistence) and the /auth · /logout · Big Pickle · mixed-row picker paths are untested.
| // Bind the port BEFORE opening the browser so the credential can | ||
| // only be delivered to this process. | ||
| const state = randomBytes(16).toString("hex") | ||
| await startCallbackServer() |
There was a problem hiding this comment.
The browser can hand back the login before we're listening for it
MAJOR — Race — pending is registered too late. startCallbackServer() binds the port and starts accepting requests here inside authorize(), but the module-level pending (state + resolve/reject) is only set later when the framework calls the returned callback() (waitForCallback, ~L114). If the browser redirects before callback() runs (an already-signed-in user gets an instant redirect, or any scheduling gap), /callback sees pending === undefined and rejects as "Invalid state — possible CSRF", silently dropping the credential. The singleton also can't support two concurrent /auth flows — the second overwrites the first.
Fix: register pending keyed by state (e.g. a Map) synchronously in authorize() before opening the browser; have callback() await that promise, and buffer an early callback by state until it's awaited.
| : undefined | ||
| if (model) local.model.set({ providerID: props.providerID, modelID: model }, { recent: true }) | ||
| setConnected(true) | ||
| setTimeout(() => dialog.clear(), 5000) |
There was a problem hiding this comment.
Backing out of the sign-in screen doesn't actually stop the sign-in
MAJOR — No cancellation/cleanup when the dialog is dismissed. Pressing esc (or unmounting) only calls dialog.clear(); it never cancels the pending callback, so the loopback server can stay open for ~5 minutes and a late browser callback can still mutate app state after dismissal. This success-path setTimeout(() => dialog.clear(), 5000) is also never retained or cleared on unmount, so it can fire later and dismiss an unrelated dialog opened in the meantime.
Fix: guard post-await UI updates with an onCleanup disposed flag, clearTimeout in onCleanup, and stop the callback server / abort the SDK callback when the dialog is dismissed.
| let pending: Pending | undefined | ||
|
|
||
| async function startCallbackServer(): Promise<void> { | ||
| if (server) return |
There was a problem hiding this comment.
If port 7317 is busy, sign-in silently hangs forever with no retry
MAJOR — Port collision leaves a broken, silently-failing server. server is assigned by createServer() before listen(). On EADDRINUSE the listen promise rejects (L94-96) but server is never reset to undefined, so this if (server) return early-returns on the next attempt and the flow proceeds as if listening — the callback never arrives and the user waits out the 5-minute timeout with no message.
Fix: in the error handler set server = undefined (and close it), and surface a clear "port 7317 in use" error to the TUI. Consider an ephemeral port if the redirect contract allows.
| <h2>Signed in ✓</h2><p>You can return to your terminal.</p> | ||
| <script>setTimeout(()=>window.close(),1500)</script></body>` | ||
|
|
||
| const HTML_ERROR = (msg: string) => `<!doctype html><meta charset="utf-8"><title>Altimate Code</title> |
There was a problem hiding this comment.
The error page echoes attacker-controlled text straight into HTML
MAJOR — Reflected, unescaped input in the callback error page (localhost XSS). HTML_ERROR(msg) interpolates msg into HTML with no escaping, and the value ultimately comes from url.searchParams (the error branch at L57, which runs before the state check). Any page that drives the browser to http://localhost:7317/callback?error=<script>... during the auth window yields reflected XSS on the localhost origin. Impact is limited (transient page, no secrets/cookies there) but it's avoidable.
Fix: HTML-escape all reflected values, and validate state before handling the error branch.
| }) | ||
|
|
||
| await new Promise<void>((resolve, reject) => { | ||
| server!.listen(CALLBACK_PORT, () => resolve()) |
There was a problem hiding this comment.
The sign-in callback server is reachable from the whole network, not just this machine
MAJOR — Callback server listens on all interfaces, not loopback. listen(CALLBACK_PORT) with no host binds to ::/0.0.0.0, so the auth callback server is reachable from the LAN — despite the comment claiming "the credential can only be delivered to this process." The 128-bit state still protects credential delivery, but the listener still processes remote requests (probing, plus the pre-state-check error abort below).
Fix: server!.listen(CALLBACK_PORT, "127.0.0.1", () => resolve()).
| // A provider is "ready" (usable now) when it has valid credentials: it is present | ||
| // in the live provider list with at least one model — and, for the free OpenCode | ||
| // provider, with at least one paid model (a Zen key entered). | ||
| function providerReady(id: string) { |
There was a problem hiding this comment.
A provider with unknown pricing can be treated as "ready" without a key
MINOR — providerReady("opencode") mishandles undefined cost. m.cost?.input !== 0 evaluates to true when cost is undefined, so OpenCode can be counted as "ready" (usable now) even without a Zen key entered.
Fix: m.cost?.input != null && m.cost.input !== 0. (Same defect exists in useConnected() — see the separate comment.)
| // Persist to ~/.altimate/altimate.json — the provider loader | ||
| // reads this first (it carries the instance/tenant + api_url | ||
| // the generic auth.json store can't). | ||
| await AltimateApi.saveCredentials({ |
There was a problem hiding this comment.
Browser sign-in skips the tenant-name check that manual entry enforces
MINOR — OAuth callback skips the tenant-name validation the manual path enforces. The manual paste path runs validateCredentials() (tenant regex + API check) before saving; this OAuth path calls saveCredentials() directly on the callback instance. The source is the trusted web app, so risk is low, but the invariant differs between the two entry points.
Fix: validate instance against VALID_TENANT_REGEX before persisting, or document why the OAuth source is trusted.
| // useConnected() (real credentials) via useReady(), it gates the first-run chat | ||
| // lock. Module-global so it is shared across the app and resets on every process | ||
| // launch (so a fresh relaunch is a clean fresh-user state). | ||
| const [setupComplete, setSetupComplete] = createSignal(false) |
There was a problem hiding this comment.
After signing out, the app still shows "you're all set" guidance
MINOR — setupComplete is never reset on /logout. This module-global signal is OR'd with connected() in useReady(). /logout clears credentials (so connected() → false) but leaves setupComplete === true, so useReady() stays true for the rest of the process. Verified scope: useReady() only drives tips and the update-toast (not the chat/prompt), so the real impact is that the post-logout panel keeps showing "ready" tips (/discover) instead of /connect until relaunch. (Reviewers split on severity; kept minor given the cosmetic impact.)
Fix (if desired): gate the post-logout tips on connected() rather than useReady(), or add a resetSetupComplete() called from /logout.
| res.end(body) | ||
| } | ||
|
|
||
| const error = url.searchParams.get("error") |
There was a problem hiding this comment.
Anyone on the network can cancel your sign-in without any secret
MAJOR — Unauthenticated ?error= can abort an in-progress sign-in. This error branch runs before state validation and rejects/clears the current pending flow. Combined with the all-interface bind above, any local or LAN requester can cancel a user's sign-in without knowing state — there's no CSRF protection on the abort path.
Fix: validate state before honoring the error branch (this also closes the reflected-XSS issue on the same branch).
| // are used by the restructured DialogModel below. | ||
| import { markSetupComplete, DialogBigPickleConfirm } from "./altimate-onboarding" | ||
|
|
||
| export function useConnected() { |
There was a problem hiding this comment.
The same unknown-pricing bug also mislabels providers as connected
MINOR — useConnected() has the same undefined-cost defect as providerReady(). It treats OpenCode as connected when any model has cost?.input !== 0, which is also true for undefined cost. This one has a wider blast radius: it drives the suggested: !connected() flag on the /connect and /auth commands, not just the model picker.
Fix: apply the same != null && !== 0 guard here.
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Requesting changes — PR #1001
Consensus review by 6 models (Claude, GPT 5.4 Codex, Kimi K2.5, MiniMax M2.7, Qwen 3.6, MiMo V2.5 Pro), converged and re-validated line-by-line against the code. Formalizing the verdict from the inline review above (#pullrequestreview-4705485161).
7 major, 9 minor findings — all in the new OAuth loopback auth path. The design is sound (crypto-random state, CSRF check, 0o600 credentials, cleanly isolated onboarding), but the loopback server has correctness/reliability bugs and localhost hardening gaps that should be fixed before merge.
Blocking (major):
- Race —
pendingregistered after the server starts accepting requests; an instant redirect drops the credential (altimate.ts:140). - Dismissing the dialog doesn't cancel the flow; server +
setTimeoutleak (dialog-provider.tsx:209). - Port collision (
EADDRINUSE) leavesserverset → silent 5-min hang on retry (altimate.ts:43). - Reflected, unescaped
errorin the callback HTML — localhost XSS (altimate.ts:23). - Callback server binds all interfaces instead of
127.0.0.1(altimate.ts:94). - API key delivered in the URL query string (
altimate.ts:76). - Unauthenticated
?error=aborts an in-progress sign-in without knowingstate(altimate.ts:57).
Minor findings and fixes are in the inline comments. Nothing here blocks the onboarding UX direction — the issues are contained to auth-path correctness, security hardening, and error handling. Happy to walk through any of them.
2 participants
Summary
CLI onboarding for the Altimate LLM Gateway + a welcoming first-run experience, plus quick sign-in/out commands.
Gateway sign-in
:7317) + browser → web sign-up page → credential delivered back to the CLI; saved to~/.altimate/altimate.json(overridable viaALTIMATE_WEB_URL).First-run experience
WelcomePanelboot box on the home screen with readiness-aware tips (/connect→/discover); update toast suppressed until a model is ready.useReady/markSetupCompletegate the first-run chat lock (unlocks after connecting, choosing Big Pickle, or picking a ready model).Provider picker
/connectopens the curatedDialogModelWelcome— top 5 providers (Altimate Gateway first) + "Search all providers…", a Big Pickle fallback, and a bright-green ✓ "selected" marker on the currently-active provider/model./modeldialog is restructured into READY / NEEDS-SETUP sections.New TUI commands
/auth— start the Altimate LLM Gateway sign-in directly (DialogAltimateAuth)./logout— clear the saved gateway credential (AltimateApi.clearCredentials), disconnect, and reload.Rebase safety
component/altimate-onboarding.tsx;dialog-model.tsxtrimmed to pristineuseConnected+ the markedDialogModelrestructure, so future upstream (opencode) merges have a minimal, clearly-marked conflict surface. No behavior change.Test plan
/connect→ Altimate LLM Gateway → browser → provisioned + connected/agents/v1/models→200)/authlaunches gateway sign-in;/logoutclears creds + disconnectsbun run typecheckclean🤖 Generated with Claude Code