Repository navigation
fix(onboarding): unbreak the first API call - #3524
Merged
Merged
Conversation
The try-it request lost its session cookie when it moved to the API's `/chat/completion` proxy, so every new account's first call 401'd. Send credentials, normalize GATEWAY_URL's `/v1` suffix, and surface streamed gateway errors instead of swallowing them as "No response received". New accounts have no credits, so the proxy now sponsors that one call with a platform key (`ONBOARDING_CHAT_API_KEY`), pinning the model and an output cap server-side and only while onboarding is incomplete. The free Claude Haiku 4.5 mapping it used to rely on is deactivated. Adds a Playwright suite for apps/ui covering signup through onboarding. Claude-Session: https://claude.ai/code/session_0174tWJnTyV1nd94FDDUAX1s
Contributor
WalkthroughThe PR adds authenticated onboarding chat sponsorship with a fixed model and token limit. It updates the onboarding UI and adds API, browser, stream-parser, and gateway URL tests. It also removes free-model snippet options and deactivates a retired provider. ChangesOnboarding chat flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingWizard
participant ChatRoute
participant Redis
participant Gateway
User->>OnboardingWizard: Submit onboarding prompt
OnboardingWizard->>ChatRoute: Send session-authenticated request
ChatRoute->>Redis: Consume sponsorship allowance
ChatRoute->>Gateway: Send sponsored completion
Gateway-->>ChatRoute: Return SSE stream
ChatRoute-->>OnboardingWizard: Forward completion stream
OnboardingWizard-->>User: Render content or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/onboarding/onboarding-wizard.tsx (1)
126-148: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the generated API client for the onboarding chat proxy.
apps/ui/src/components/onboarding/onboarding-wizard.tsx:129callsPOST /chat/completionwith rawfetch(). UseuseFetchClient()and preserve thecredentials: "include"setting and SSE/stream content/error handling the wizard already implements. The generated client does not need a new endpoint; it should call the existing proxy route through the API surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ui/src/components/onboarding/onboarding-wizard.tsx` around lines 126 - 148, The onboarding chat request in the wizard’s fetch flow should use the generated API client via useFetchClient() instead of raw fetch to call the existing chat completion proxy. Preserve credentials: "include", request payload behavior, AbortSignal support, and the existing SSE stream parsing and error handling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/chat.spec.ts`:
- Around line 25-29: Update the test setup and fallback test around
createTestUser and getOnboardingApiKey to preserve both ONBOARDING_CHAT_API_KEY
and SUPPORT_CHAT_API_KEY. Stub or save each variable before tests, clear both
when exercising onboarding-key fallback, and restore their original values
automatically after each test so pre-existing environment configuration is not
lost.
In `@apps/api/src/routes/chat.ts`:
- Around line 66-97: Update the sponsorship flow in the chat route around the
sponsored calculation and before the gateway fetch to enforce a server-side
per-user or per-organization allowance. Atomically reserve or consume the
allowance before forwarding sponsored requests, deny sponsorship when it is
exhausted, and preserve the existing API-key requirement and onboarding
model/token limits for allowed requests.
In `@apps/api/src/utils/playground-key.spec.ts`:
- Around line 9-16: Update the afterEach cleanup to restore NODE_ENV using the
same delete-or-restore pattern as GATEWAY_URL: delete process.env.NODE_ENV when
previousNodeEnv is undefined; otherwise assign the saved value.
In `@apps/api/src/utils/playground-key.ts`:
- Line 18: Update the URL normalization expression in the playground-key utility
to remove all repeated terminal `/v1` suffixes, while preserving the final
single `/v1` suffix and existing trailing-slash handling. Add a regression test
covering `https://gateway.example/v1/v1` and verify it produces a URL ending in
exactly `/v1`.
In `@apps/ui/src/components/onboarding/onboarding-wizard.tsx`:
- Around line 219-224: Update the stream-error handling around the streamError
check to setTryError and capture onboarding_try_error whenever streamError
exists, regardless of accumulated content. Preserve partial output visibility,
and add coverage for content followed by an SSE error event to ensure
onboarding_try_success is not recorded.
---
Outside diff comments:
In `@apps/ui/src/components/onboarding/onboarding-wizard.tsx`:
- Around line 126-148: The onboarding chat request in the wizard’s fetch flow
should use the generated API client via useFetchClient() instead of raw fetch to
call the existing chat completion proxy. Preserve credentials: "include",
request payload behavior, AbortSignal support, and the existing SSE stream
parsing and error handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fbe66d90-ea5d-453d-9600-2344bfb6a75d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/api/src/routes/chat.spec.tsapps/api/src/routes/chat.tsapps/api/src/utils/playground-key.spec.tsapps/api/src/utils/playground-key.tsapps/ui/.gitignoreapps/ui/e2e/onboarding.pw.tsapps/ui/package.jsonapps/ui/playwright.config.tsapps/ui/src/components/onboarding/onboarding-wizard.tsxapps/ui/src/components/shared/quick-start-snippet.tsxapps/ui/tsconfig.jsonpackages/models/src/models/anthropic.tspackages/shared/src/index.tspackages/shared/src/onboarding.ts
💤 Files with no reviewable changes (1)
- apps/ui/src/components/shared/quick-start-snippet.tsx
Sponsored onboarding calls now consume a per-user Redis allowance, so an account that never finishes onboarding can't loop on the platform key. Collapse repeated `/v1` suffixes in getGatewayUrl, and restore NODE_ENV and both platform-key env vars in the specs instead of clobbering them. The wizard now calls the proxy through the generated typed client (parseAs: "stream" keeps the SSE body unread) and reports a mid-stream error even when partial content arrived, so a failed call is never recorded as a success. Stream parsing moves to a tested helper. Claude-Session: https://claude.ai/code/session_0174tWJnTyV1nd94FDDUAX1s
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/ui/src/components/onboarding/completion-stream.ts (1)
72-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the parsed SSE chunk instead of relying on the implicit
anyfromJSON.parse.
JSON.parsereturnsany, soparsed.error,parsed.choices, andparsed.modelare unchecked. A non-stringmodelvalue would flow intoCompletionStreamResult.modeland then into the PostHogonboarding_try_successevent inapps/ui/src/components/onboarding/onboarding-wizard.tsxat Line 186. A narrow local type keeps the reader strict without adding validation cost.♻️ Proposed typed chunk shape
+interface CompletionChunk { + error?: unknown; + model?: unknown; + choices?: { delta?: { content?: unknown } }[]; +} + try { - const parsed = JSON.parse(data); + const parsed = JSON.parse(data) as CompletionChunk; if (parsed.error) { error = extractErrorMessage(parsed); } const delta = parsed.choices?.[0]?.delta?.content; - if (delta) { + if (typeof delta === "string" && delta) { content += delta; onContent(content); } - if (parsed.model) { + if (typeof parsed.model === "string") { model = parsed.model; } } catch {As per coding guidelines: "Use strict TypeScript; never use
anyoras anyunless absolutely necessary".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ui/src/components/onboarding/completion-stream.ts` around lines 72 - 84, Define a narrow local TypeScript type for the parsed SSE chunk, including the optional error, choices/delta content, and string model fields, and parse JSON into that type instead of relying on JSON.parse’s implicit any. Use the typed value throughout this try block so parsed.error, parsed.choices, and parsed.model remain strictly checked before updating completion state.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/ui/src/components/onboarding/completion-stream.ts`:
- Around line 72-84: Define a narrow local TypeScript type for the parsed SSE
chunk, including the optional error, choices/delta content, and string model
fields, and parse JSON into that type instead of relying on JSON.parse’s
implicit any. Use the typed value throughout this try block so parsed.error,
parsed.choices, and parsed.model remain strictly checked before updating
completion state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95bba8e0-b324-4fdd-81f9-d0a9ca550ae0
📒 Files selected for processing (7)
apps/api/src/routes/chat.spec.tsapps/api/src/routes/chat.tsapps/api/src/utils/playground-key.spec.tsapps/api/src/utils/playground-key.tsapps/ui/src/components/onboarding/completion-stream.spec.tsapps/ui/src/components/onboarding/completion-stream.tsapps/ui/src/components/onboarding/onboarding-wizard.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/utils/playground-key.ts
- apps/api/src/utils/playground-key.spec.ts
- apps/api/src/routes/chat.ts
- apps/api/src/routes/chat.spec.ts
smakosh
enabled auto-merge (squash)
August 10, 2026 18:54
smakosh
added a commit
that referenced
this pull request
Aug 11, 2026
## Problem The post-signup onboarding wizard's "try it now" request does not show up in the account's own activity log. The user sends a real request, sees a real streamed answer, and then finds an empty dashboard. `#3524` sponsored that first call with a platform-owned key (`ONBOARDING_CHAT_API_KEY`), because a brand new organization is created with 0 credits and a paid model would `402`. But the gateway derives *everything* about ownership from the authenticating key — `apps/gateway/src/chat/chat.ts` writes every log row with `projectId: apiKey.projectId` and `organizationId: project.organizationId`. So the row landed in the platform org's project, and `GET /logs` can never return it: it scopes to `getActiveUserOrganizationIds(user.id)`. This is deterministic, not a race or a worker delay — it happens on every signup wherever the platform key is configured. In production the missing rows are all sitting in the org that owns that key, filterable with `source=onboarding`. Ruled out along the way: - **Retention.** New orgs default to `retentionLevel: "none"`, but `insertLog` only strips payload fields in that case — the row is still inserted. - **Wrong project for the displayed key.** `/playground/ensure-key` creates the key in the user's own default project. - **Email verification.** `onboarding: true` already skips the free-model gate. ## Approach **The call now always runs on the account's own API key**, so logs, usage and billing attribute correctly by construction rather than being reconstructed after the fact. That leaves the original problem: a new org has 0 credits. Rather than paying for the call with a platform key (which is what broke attribution) or granting credits, **that one call is zero-rated** — the credit gate lets it through and the computed cost is zeroed, so nothing is ever debited and no credits change hands. Zero-rating rather than granting is deliberate. Credits are fungible and bankable: a grant is worth its face value to a signup farmer, can be spent on anything, and lands in every credit metric (`totalToppedUp` / `unusedCredits` sum all completed transactions). A zero-rated call is one capped completion that cannot be saved, redirected or spent on anything else. ### How it is authorized Eligibility is decided in the API proxy, where the session lives — signed in, onboarding not yet complete, allowance not spent — and asserted to the gateway with a shared secret. The `onboarding` body flag deliberately cannot authorize this: it is client-supplied, and honoring it would let anyone waive their own charges. ### Notable details - **Costs are zeroed in one wrapper** around `calculateCosts`, not at each of the ~11 places costs are computed (streaming, non-streaming, cached, cancelled, content-filter, error paths). The import is renamed to `_calculateCosts`, so any billing path outside that wrapper's scope **fails to compile** instead of silently charging the user for the first thing they ever did. - **Storage cost needs no special handling**: `calculateDataStorageCost` returns `"0"` when `retentionLevel === "none"`, which is the default for a new org. Nothing debits it, so the balance stays at exactly 0 rather than going negative. - **The allowance is reserved before the call and refunded when the gateway never served it**, so a flaky provider can't exhaust onboarding and leave the user on a bare `402`. The refund is guarded with a Lua `exists` check — a bare `DECR` on an expired counter would recreate it at `-1` with no TTL and hand out an extra call next window. - **Onboarding runs on `"auto"`** so it never pins a model that later gets retired or repriced. - Without the secret configured (self-hosted, local dev) nothing is zero-rated and the call is billed normally — it degrades to pre-`#3524` behavior rather than silently giving work away. ## Regression cover The Playwright suite now asserts **the thing that was actually broken**: after the first call it polls `/logs?source=onboarding` until the request appears in the new account's own activity. Since `/logs` only ever returns the caller's own organizations, that assertion is precisely the regression — it fails against the platform-key implementation. - `apps/gateway/src/lib/onboarding-sponsorship.spec.ts` — the secret check, weighted to the negative cases: no header, wrong secret, a prefix of the secret (`timingSafeEqual` throws on length mismatch), and nothing configured. - `apps/api/src/routes/chat.spec.ts` — the request always leaves on the caller's own key; sponsorship is asserted only while eligible; it stops after onboarding completes, when no secret is set, and once the allowance is spent; a failed gateway call refunds the allowance. ## Test plan - `pnpm build` ✅ - `pnpm format` ✅ - `pnpm test:unit` against an isolated worktree stack — **4449 passed**, 2 skipped ✅ - One unrelated pre-existing failure in `apps/gateway/src/chat/tools/openai-content-filter.spec.ts` ("logs missing moderation credentials"), confirmed by reverting this branch's gateway changes and reproducing it identically. It is sensitive to whether `LLM_OPENAI_API_KEY` is present locally. - `pnpm --filter ui test:e2e` requires the worker running, since log rows reach the database through the log queue. ## Deploy note Set **`ONBOARDING_SPONSOR_SECRET`** to a random secret shared by the API and the gateway. Without it, onboarding's first call is billed to the new account and will `402`. **`ONBOARDING_CHAT_API_KEY`** (added by `#3524`) is no longer read anywhere and can be removed from the deploy config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Eligible onboarding requests can proceed without consuming account credits when authorized by the configured sponsorship secret. * Onboarding usage remains recorded in account activity with zero inference and storage costs. * Onboarding now uses the pinned DeepSeek model, with a 4,000-character prompt limit and 512-token output limit. * **Bug Fixes** * Failed, canceled, streaming, cached, malformed, and filtered requests no longer consume sponsorship allowances. * Sponsorship requires the caller’s API key and server authorization; the client-provided onboarding flag no longer grants access. * Normal plan, model, and free-model restrictions remain enforced for sponsored requests. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The post-signup onboarding wizard's "try it now" request has been failing for every new account.
#3183moved that request off the gateway and onto the API's server-side proxy (POST /chat/completion), because the gateway no longer accepts cross-origin browser calls. That proxy sits behind the session middleware, but the fetch was never givencredentials: "include"— the API is on a different origin than the UI, so the browser dropped the session cookie and the call401'd. The wizard rendered it as a bare "Request failed".Two things were hiding behind it:
getGatewayUrl()returnedGATEWAY_URLverbatim while every caller appends/v1paths.GATEWAY_URLis written both ways across environments (the frontends strip the suffix,AGENTS.md's isolated-stack recipe omits it), so a stack configured without it404'd against the gateway.errorevent as success — the gateway reports post-headers failures with a200— so a real upstream error surfaced as "No response received" and counted as a completed first call.Email verification, which is what prompted this, turned out not to be the blocker:
onboarding: truealready skips the free-model verification gate (verified against the gateway with an unverified account —403without the flag, through with it).Changes
Fix the first call
useFetchClient()), which sends the session cookie;parseAs: "stream"hands back the SSE body unread so streaming still works.GATEWAY_URLto exactly one/v1suffix ingetGatewayUrl().onboarding_try_success. Stream parsing moved to a tested helper (completion-stream.ts).Sponsor the first call
A brand new organization is created with 0 credits, so pointing onboarding at a paid model would
402for every signup.POST /chat/completionnow sponsors that one call with a platform key (ONBOARDING_CHAT_API_KEY, falling back toSUPPORT_CHAT_API_KEY). Because we pay for it, the sponsored path:ONBOARDING_MODEL=runware/gpt-oss-120b) rather than trusting the request body,x-source: onboarding.Without a platform key configured (self-hosted, local dev) it falls back to the caller's own key, exactly as before.
Retire the free model
claude-haiku-4-5-free's Anthropic mapping is deactivated — it ran on our own Anthropic credentials and onboarding was the last surface that needed a free model. It disappears from/models(the directory hides deactivated mappings by default). The free-model email-verification gate andfree_models_onlyremain in place but have no active model to select, so the Quick Start snippets no longer passfree_models_only.Regression cover
apps/ui(pnpm --filter ui test:e2e): signs up a fresh account, asserts onboarding isn't complete, waits on the realPOST /chat/completion, and fails on any gating error (401, missing credits, email verification) while tolerating an upstream provider error so a local stack without the model'sLLM_*key still runs. Then finishes the wizard and assertsonboardingCompleted. A second test asserts the proxy still rejects an unauthenticated call.apps/api/src/routes/chat.spec.tspins the sponsorship rules: session required, platform key used, model and cap pinned, sponsorship stops after onboarding completes, and the fallback to the caller's key.apps/api/src/utils/playground-key.spec.tspins the/v1normalization.apps/ui/src/components/onboarding/completion-stream.spec.tscovers SSE parsing: content, chunk-boundary splits, malformed chunks, and an error arriving after partial content.Screenshots
/onboardingafter clicking Send request.Before — the call never reaches the gateway.
After — a real streamed answer, on a brand new account with no credits.
Deploy note
Set
ONBOARDING_CHAT_API_KEYto a platform-owned gateway key (same pattern asSUPPORT_CHAT_API_KEY, which it falls back to). Without it, onboarding's first call runs on the new account's own key and will402on a paid model.Test plan
pnpm build✅pnpm vitest run apps/api/src/routes/chat.spec.ts apps/api/src/utils/playground-key.spec.ts packages/models packages/shared— 398 passed ✅pnpm vitest run apps/gateway/src/api.spec.ts apps/gateway/src/dynamic-routes.spec.ts— 161 passed ✅pnpm --filter ui test:e2eagainst a local stack — 2 passed ✅parseAs: "stream"confirmed not to consume the body)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests