feat(onboarding): first-run experience for third-party providers (#1864)

* feat(onboarding): first-run experience for third-party providers

Two gates in showSetupScreens were keyed on usesAnthropicAccountFlow(),
so users of any non-Anthropic provider skipped onboarding entirely:

- Onboarding (theme + security notes) now runs for all providers. The
  component already drops its preflight/OAuth steps when Anthropic auth
  is not enabled, so third-party users get theme -> security notes ->
  terminal setup with no login screens.
- The trust dialog now runs for all providers. Workspace trust is
  orthogonal to the API provider — an untrusted repo is exactly as
  dangerous over a local model as over Anthropic. (The block comment
  even said "always show"; the inner gate contradicted it.)

Also: the login-method screen now detects OPENAI_BASE_URL+OPENAI_MODEL
in the environment and offers "Use current environment configuration"
as the first (default) option. Selecting it saves and activates a
provider profile via addProviderProfile — env vars alone do NOT
activate the OpenAI route (resolveActiveRouteIdFromEnv requires
CLAUDE_CODE_USE_OPENAI or a saved profile), a gap previously masked in
manual testing by a stray legacy .openclaude-profile.json in the cwd.

Verified live (tmux, scratch config dir, mock OpenAI server):
fresh 3P first run walks theme -> security -> trust -> REPL; env
option saves "Local OpenAI-compatible", the session completes a real
turn against the env endpoint, and the profile persists across
relaunch. Second launch shows no onboarding.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>

* fix(onboarding): address review — testable gating seam + env-profile dedup

- The first-run screen decisions move into src/utils/setupScreenGates.ts,
  a provider-free importable seam (showSetupScreens' import chain cannot
  be loaded under bun test — the same constraint and pattern as the
  dev-channels registration seam). Behavioral tests cover the gate matrix
  (fresh install, completed install, theme-missing re-show, trust
  independence, claubbit skip); the bugfixes.test.ts checks now assert
  the wiring (both dialogs consult the seam, no provider gate at the
  call sites) instead of only regexing for the removed string.
- The "use current environment configuration" onboarding option dedupes:
  an existing profile matching the env base URL + model is re-activated
  via setActiveProviderProfile (which also re-applies profile env and
  syncs the startup profile file) instead of appending a near-identical
  profile on every pass through the flow.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>

* fix(onboarding): refresh reused profile credentials + accurate env var label

- The reuse branch now refreshes the stored credential from the
  environment before activating: a rotated OPENAI_API_KEY would otherwise
  leave the flow running on the profile's stale key. Falls back to the
  existing key when the env no longer carries one, so a working
  credential is never blanked. Status text says "Activated" for reuse and
  keeps "Saved" for a newly created profile.
- The environment option's label names the variable the value actually
  came from (OPENAI_BASE_URL vs OPENAI_API_BASE) instead of hardcoding
  the former, so troubleshooting points at a variable that is really set.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>

* fix(onboarding): preserve profile fields and verify the credential refresh

Follow-up on the reuse path added last round:

- updateProviderProfile REPLACES the profile (toProfile builds a fresh
  object rather than merging), so passing only name/baseUrl/model/apiKey
  silently dropped any configured apiFormat, azureStyle, authHeader,
  authScheme, authHeaderValue, customHeaders, or maxContextLength. Spread
  the existing profile and override only the refreshed credential.
- A null return from updateProviderProfile (env values failing profile
  validation) no longer falls through to activation: reporting
  "Activated" while still running on the stale key is worse than routing
  the user to guided setup, which is what the create path already does.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>

* fix(onboarding): redact credential-bearing endpoints before display

OPENAI_BASE_URL / OPENAI_API_BASE are credential-bearing in the wild
(userinfo like https://user:pass@host/v1, or ?token=/?api_key= query
params), and both the option label and the completion status message
rendered the raw value straight into terminal scrollback.

The derivation moves into src/utils/envProviderOption.ts, which owns the
disclosure boundary explicitly: `displayBaseUrl` is passed through the
codebase's existing redactUrlForDisplay and is the only form the UI may
render, while the raw `baseUrl` is retained for profile creation and
activation so the saved profile still authenticates. Both rendered sites
now use the redacted value.

Regression coverage: envProviderOption.test.ts asserts userinfo and
sensitive query params never reach displayBaseUrl (including via the
non-URL fallback path) while baseUrl stays intact, plus var-name and
availability cases; a wiring guard in bugfixes.test.ts fails if either
rendered site is ever pointed back at the raw endpoint.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>

---------

Co-authored-by: OpenClaude <openclaude@gitlawb.com>
This commit is contained in:
Kevin Codex
2026-07-20 16:23:14 +08:00
committed by GitHub
co-authored by OpenClaude
parent 83d54b0ac8
commit fff83a1a7f
7 changed files with 405 additions and 6 deletions
+52
View File
@@ -695,3 +695,55 @@ describe('Dev-channels dialog coverage', () => {
)
})
})
// ---------------------------------------------------------------------------
// Fix: onboarding + trust dialog skipped entirely for third-party providers
// ---------------------------------------------------------------------------
// Behavioral coverage lives in src/utils/setupScreenGates.test.ts — the
// gating decisions were extracted into that provider-free seam because this
// module's import chain cannot be loaded under bun test (compile-time
// feature() macro checker, same constraint as the dev-channels tests above).
// These wiring checks assert showSetupScreens actually consults the seam and
// that no provider gate was re-introduced around the dialogs.
describe('Onboarding and trust dialog — third-party providers', () => {
test('showSetupScreens routes both dialogs through the provider-free seam', async () => {
const content = await file('interactiveHelpers.tsx').text()
expect(content).toContain('getRequiredSetupScreens({')
expect(content).toContain('if (setupScreens.onboarding)')
expect(content).toContain('if (setupScreens.trustDialog)')
})
test('the env-config option never renders the raw endpoint', async () => {
// OPENAI_BASE_URL/OPENAI_API_BASE can carry credentials (userinfo or
// token query params) and everything rendered lands in terminal
// scrollback. Redaction behavior is tested in envProviderOption.test.ts;
// this guards the wiring — the raw `envBaseUrl` may only reach profile
// persistence (addProviderProfile/getProviderProfiles/label), never a
// rendered label or status message.
const content = await file('components/ConsoleOAuthFlow.tsx').text()
expect(content).toContain('getEnvProviderOption()')
// Rendered sites use the redacted value.
expect(content).toMatch(/\{envBaseUrlVarName\}=\{envBaseUrlForDisplay\}/)
expect(content).toMatch(/\$\{envBaseUrlForDisplay\}\) as your active provider/)
// No rendered site interpolates the raw endpoint.
expect(content).not.toMatch(/\{envBaseUrl\}/)
expect(content).not.toMatch(/\$\{envBaseUrl\}/)
})
test('no dialog is gated behind usesAnthropicSetup', async () => {
const content = await file('interactiveHelpers.tsx').text()
// Theme choice + security notes are universal, and workspace trust is
// orthogonal to the API provider: an untrusted repo is exactly as
// dangerous over a local model as over Anthropic. The seam takes no
// provider input, so the only way to regress is to add a gate at the
// call sites — which this guards against.
expect(content).not.toMatch(/usesAnthropicSetup\s*&&\s*\(?\s*setupScreens/)
expect(content).not.toMatch(/usesAnthropicSetup\s*&&\s*\(\s*!config\.theme/)
expect(content).not.toMatch(
/usesAnthropicSetup\s*&&\s*!checkHasTrustDialogAccepted/,
)
})
})
+106
View File
@@ -11,6 +11,15 @@ import { sendNotification } from '../services/notifier.js';
import { OAuthService } from '../services/oauth/index.js';
import { getOauthAccountInfo, validateForceLoginOrg } from '../utils/auth.js';
import { logError } from '../utils/log.js';
import { getEnvProviderOption } from '../utils/envProviderOption.js';
import { getLocalOpenAICompatibleProviderLabel } from '../utils/providerDiscovery.js';
import { type ProviderProfile } from '../utils/config.js';
import {
addProviderProfile,
getProviderProfiles,
setActiveProviderProfile,
updateProviderProfile,
} from '../utils/providerProfiles.js';
import { getSettings_DEPRECATED } from '../utils/settings/settings.js';
import { ProviderManager } from './ProviderManager.js';
import { Select } from './CustomSelect/select.js';
@@ -386,7 +395,42 @@ function OAuthStatusMessage({
startingMessage ||
'OpenClaude can be used with your Claude subscription or billed based on API usage through your Console account.'
// OPENAI_BASE_URL/OPENAI_MODEL in the environment signal an
// OpenAI-compatible setup the user already has — offer to adopt it as
// the active provider profile instead of walking them through login for
// an account they may never have wanted. Env vars alone do NOT activate
// the route (resolveActiveRouteIdFromEnv requires CLAUDE_CODE_USE_OPENAI
// or a saved profile), so selecting this saves + activates a profile.
// Both fields gate the option because a profile requires baseUrl+model.
// getEnvProviderOption owns the secret-disclosure boundary: only
// `displayBaseUrl` (redacted) may be rendered — the raw `baseUrl`
// exists solely for profile creation/activation. See its tests for
// the credential cases.
const {
available: envConfigAvailable,
varName: envBaseUrlVarName,
baseUrl: envBaseUrl,
displayBaseUrl: envBaseUrlForDisplay,
model: envModel,
} = getEnvProviderOption()
const loginOptions = [
...(envConfigAvailable
? [
{
label: (
<Text>
Use current environment configuration ·{' '}
<Text dimColor>
{envBaseUrlVarName}={envBaseUrlForDisplay}
</Text>
{'\n'}
</Text>
),
value: 'environment' as const,
},
]
: []),
{
label: (
<Text>
@@ -427,6 +471,68 @@ function OAuthStatusMessage({
<Select
options={loginOptions}
onChange={value => {
if (value === 'environment') {
// Re-entering this flow with the same env must not stack
// near-identical profiles: reuse (and activate) an existing
// profile matching the env base URL + model before creating
// a new one. setActiveProviderProfile also re-applies the
// profile env and syncs the startup profile file.
const existing = getProviderProfiles().find(
profile =>
profile.baseUrl?.trim() === envBaseUrl?.trim() &&
profile.model?.trim() === envModel?.trim(),
)
let saved: ProviderProfile | null
if (existing) {
// Refresh the stored credential from the environment
// before activating: the env is the source of truth the
// user just chose, so a rotated OPENAI_API_KEY must not
// leave the profile running on a stale key. Keep the
// existing key when the env no longer carries one rather
// than blanking a working credential.
//
// updateProviderProfile REPLACES the profile (toProfile
// builds a fresh object, it does not merge), so spread
// the existing profile first — otherwise a configured
// apiFormat / auth header / customHeaders / context
// length would be silently dropped on refresh.
const refreshed = updateProviderProfile(existing.id, {
...existing,
apiKey: process.env.OPENAI_API_KEY ?? existing.apiKey,
})
if (!refreshed) {
// The env values failed profile validation. Activating
// now would claim a refresh that did not happen, so send
// the user to guided setup instead of silently running
// on the stale credential.
setOAuthStatus({ state: 'platform_setup' })
return
}
saved = setActiveProviderProfile(existing.id)
} else {
saved = addProviderProfile(
{
name: getLocalOpenAICompatibleProviderLabel(envBaseUrl),
baseUrl: envBaseUrl as string,
model: envModel as string,
apiKey: process.env.OPENAI_API_KEY,
},
{ makeActive: true },
)
}
if (!saved) {
// Env values failed profile validation — fall back to the
// guided provider setup with fields prefilled from env.
setOAuthStatus({ state: 'platform_setup' })
return
}
logEvent('tengu_oauth_env_config_selected', {})
setOAuthStatus({
state: 'platform_setup_complete',
message: `${existing ? 'Activated' : 'Saved'} ${saved.name} (${envBaseUrlForDisplay}) as your active provider.`,
})
return
}
if (value === 'platform') {
logEvent('tengu_oauth_platform_selected', {})
setOAuthStatus({ state: 'platform_setup' })
+20 -6
View File
@@ -20,6 +20,7 @@ import { onChangeAppState } from './state/onChangeAppState.js';
import { normalizeApiKeyForConfig } from './utils/authPortable.js';
import { getExternalClaudeMdIncludes, getMemoryFiles, shouldShowClaudeMdExternalIncludesWarning } from './utils/claudemd.js';
import { checkHasTrustDialogAccepted, getCustomApiKeyStatus, getGlobalConfig, saveGlobalConfig } from './utils/config.js';
import { getRequiredSetupScreens } from './utils/setupScreenGates.js';
import { updateDeepLinkTerminalPreference } from './utils/deepLink/terminalPreference.js';
import { isEnvTruthy, isRunningOnHomespace } from './utils/envUtils.js';
import { type FpsMetrics, FpsTracker } from './utils/fpsTracker.js';
@@ -114,9 +115,20 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod
const config = getGlobalConfig();
let onboardingShown = false;
// Skip onboarding dialog for third-party providers (no Anthropic account needed)
if (usesAnthropicSetup && (!config.theme || !config.hasCompletedOnboarding) // always show onboarding at least once
) {
// Onboarding runs for ALL providers: theme + security notes are universal,
// and the component itself drops the preflight/OAuth steps when Anthropic
// auth is not enabled (see oauthEnabled in Onboarding.tsx). Gating this on
// the Anthropic account flow left third-party users with no theme choice
// and, worse, no prompt-injection/safety notes. The decisions live in the
// provider-free setupScreenGates seam (behaviorally tested there — this
// module's import chain cannot be loaded under bun test).
const setupScreens = getRequiredSetupScreens({
theme: config.theme,
hasCompletedOnboarding: config.hasCompletedOnboarding,
trustDialogAccepted: checkHasTrustDialogAccepted(),
isClaubbit: isEnvTruthy(process.env.CLAUBBIT),
});
if (setupScreens.onboarding) {
onboardingShown = true;
const {
Onboarding
@@ -136,9 +148,11 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod
// Note: non-interactive sessions (CI/CD with -p) never reach showSetupScreens at all.
// Skip permission checks in claubbit
if (!isEnvTruthy(process.env.CLAUBBIT)) {
// Skip trust dialog UI for third-party providers (no Anthropic auth), but still
// run trust state initialization below so the REPL mounts correctly.
if (usesAnthropicSetup && !checkHasTrustDialogAccepted()) {
// The trust dialog is the workspace trust boundary — it has nothing to do
// with which API provider is configured, so it runs for third-party
// providers too (an untrusted repo is exactly as dangerous over Ollama as
// over Anthropic).
if (setupScreens.trustDialog) {
const {
TrustDialog
} = await import('./components/TrustDialog/TrustDialog.js');
+89
View File
@@ -0,0 +1,89 @@
import { describe, expect, test } from 'bun:test'
import { getEnvProviderOption } from './envProviderOption.js'
// Secret-disclosure regression coverage: OPENAI_BASE_URL / OPENAI_API_BASE
// are credential-bearing in the wild, and everything the onboarding option
// renders lands in terminal scrollback. displayBaseUrl must never carry the
// secret; baseUrl must stay intact so the saved profile still works.
describe('getEnvProviderOption — credential redaction', () => {
test('URL userinfo is redacted for display but preserved for the profile', () => {
const raw = 'https://svc-user:sup3r-s3cret@api.example.com/v1'
const option = getEnvProviderOption({
OPENAI_BASE_URL: raw,
OPENAI_MODEL: 'gpt-4o',
})
expect(option.displayBaseUrl).not.toContain('sup3r-s3cret')
expect(option.displayBaseUrl).not.toContain('svc-user')
expect(option.displayBaseUrl).toContain('api.example.com')
// The working URL is untouched — the profile must still authenticate.
expect(option.baseUrl).toBe(raw)
expect(option.available).toBe(true)
})
test('sensitive query parameters are redacted for display', () => {
const raw = 'https://api.example.com/v1?token=abcd1234secret&api_key=zzz9999'
const option = getEnvProviderOption({
OPENAI_BASE_URL: raw,
OPENAI_MODEL: 'gpt-4o',
})
expect(option.displayBaseUrl).not.toContain('abcd1234secret')
expect(option.displayBaseUrl).not.toContain('zzz9999')
expect(option.displayBaseUrl).toContain('api.example.com')
expect(option.baseUrl).toBe(raw)
})
test('a credential-bearing OPENAI_API_BASE is redacted and named correctly', () => {
const raw = 'https://user:pass@gateway.internal:8443/v1'
const option = getEnvProviderOption({
OPENAI_API_BASE: raw,
OPENAI_MODEL: 'llama3',
})
expect(option.varName).toBe('OPENAI_API_BASE')
expect(option.displayBaseUrl).not.toContain('pass')
expect(option.baseUrl).toBe(raw)
})
test('a plain endpoint passes through unchanged', () => {
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'http://localhost:11434/v1',
OPENAI_MODEL: 'llama3',
})
expect(option.displayBaseUrl).toContain('localhost:11434')
expect(option.varName).toBe('OPENAI_BASE_URL')
})
test('a malformed endpoint still does not leak userinfo', () => {
// redactUrlForDisplay has a non-URL fallback path; the option must not
// regress to echoing the raw string when parsing fails.
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'not a url://user:secret-pw@host/v1',
OPENAI_MODEL: 'gpt-4o',
})
expect(option.displayBaseUrl).not.toContain('secret-pw')
})
})
describe('getEnvProviderOption — availability and var naming', () => {
test('OPENAI_BASE_URL wins over OPENAI_API_BASE and is named as such', () => {
const option = getEnvProviderOption({
OPENAI_BASE_URL: 'https://primary.example.com/v1',
OPENAI_API_BASE: 'https://fallback.example.com/v1',
OPENAI_MODEL: 'gpt-4o',
})
expect(option.baseUrl).toBe('https://primary.example.com/v1')
expect(option.varName).toBe('OPENAI_BASE_URL')
})
test('a profile needs both a base URL and a model', () => {
expect(
getEnvProviderOption({ OPENAI_BASE_URL: 'https://x.example/v1' }).available,
).toBe(false)
expect(getEnvProviderOption({ OPENAI_MODEL: 'gpt-4o' }).available).toBe(false)
expect(getEnvProviderOption({}).available).toBe(false)
})
})
+40
View File
@@ -0,0 +1,40 @@
import { redactUrlForDisplay } from './redaction.js'
/**
* Derivation for the "use current environment configuration" onboarding
* option (ConsoleOAuthFlow), extracted as a pure seam so the
* secret-disclosure boundary is regression-tested directly rather than
* asserted against component source text.
*
* The split between `baseUrl` and `displayBaseUrl` is load-bearing:
* OPENAI_BASE_URL / OPENAI_API_BASE are credential-bearing in the wild
* (userinfo like https://user:pass@host/v1, or ?token=/?api_key= query
* params). Anything rendered lands in terminal scrollback, so ONLY
* `displayBaseUrl` may reach the UI; `baseUrl` keeps the working URL for
* profile creation/activation.
*/
export type EnvProviderOption = {
/** True when both a base URL and a model are present (a profile needs both). */
available: boolean
/** The env var the base URL actually came from, for accurate troubleshooting. */
varName: 'OPENAI_BASE_URL' | 'OPENAI_API_BASE'
/** Raw endpoint — for profile persistence/activation only, never rendered. */
baseUrl: string | undefined
/** Redacted endpoint — the only form safe to render. */
displayBaseUrl: string | undefined
model: string | undefined
}
export function getEnvProviderOption(
processEnv: NodeJS.ProcessEnv = process.env,
): EnvProviderOption {
const baseUrl = processEnv.OPENAI_BASE_URL ?? processEnv.OPENAI_API_BASE
const model = processEnv.OPENAI_MODEL
return {
available: Boolean(baseUrl && model),
varName: processEnv.OPENAI_BASE_URL ? 'OPENAI_BASE_URL' : 'OPENAI_API_BASE',
baseUrl,
displayBaseUrl: baseUrl ? redactUrlForDisplay(baseUrl) : baseUrl,
model,
}
}
+69
View File
@@ -0,0 +1,69 @@
import { describe, expect, test } from 'bun:test'
import { getRequiredSetupScreens } from './setupScreenGates.js'
// Behavioral coverage for the first-run screen gating (#1864). The seam is
// deliberately provider-free — there is no input to vary by provider, which
// IS the fix: a third-party (or any) provider gets the same onboarding and
// workspace-trust decisions as the Anthropic account flow.
// showSetupScreens itself cannot be imported under bun test (its import
// chain trips the compile-time feature() macro checker), so the wiring is
// asserted structurally in src/__tests__/bugfixes.test.ts.
describe('getRequiredSetupScreens', () => {
const completed = {
theme: 'dark',
hasCompletedOnboarding: true,
trustDialogAccepted: true,
isClaubbit: false,
}
test('fresh install shows both screens', () => {
expect(
getRequiredSetupScreens({
theme: undefined,
hasCompletedOnboarding: undefined,
trustDialogAccepted: false,
isClaubbit: false,
}),
).toEqual({ onboarding: true, trustDialog: true })
})
test('fully set-up install shows neither', () => {
expect(getRequiredSetupScreens(completed)).toEqual({
onboarding: false,
trustDialog: false,
})
})
test('onboarding re-shows when the theme is missing even if completed once', () => {
expect(
getRequiredSetupScreens({ ...completed, theme: undefined }).onboarding,
).toBe(true)
})
test('onboarding re-shows when never completed even with a theme set', () => {
expect(
getRequiredSetupScreens({ ...completed, hasCompletedOnboarding: false })
.onboarding,
).toBe(true)
})
test('trust dialog shows whenever unaccepted, independent of onboarding state', () => {
expect(
getRequiredSetupScreens({ ...completed, trustDialogAccepted: false })
.trustDialog,
).toBe(true)
})
test('claubbit skips the trust dialog but never onboarding', () => {
const result = getRequiredSetupScreens({
theme: undefined,
hasCompletedOnboarding: false,
trustDialogAccepted: false,
isClaubbit: true,
})
expect(result.trustDialog).toBe(false)
expect(result.onboarding).toBe(true)
})
})
+29
View File
@@ -0,0 +1,29 @@
/**
* Pure gating decisions for the first-run setup screens, extracted from
* showSetupScreens (interactiveHelpers.tsx) as an importable seam:
* interactiveHelpers cannot be imported in tests — its import chain trips
* Bun's compile-time feature() macro checker before mocks can intercept —
* so behavioral coverage lives against this module instead (the same pattern
* as the dev-channels registration seam).
*
* Deliberately provider-free: NO input carries which API provider is active.
* That absence is the fix (#1864) — onboarding (theme + safety notes) is
* universal, with Onboarding.tsx itself dropping the OAuth/preflight steps
* when Anthropic auth is off, and workspace trust is exactly as load-bearing
* over a local model as over Anthropic. Re-introducing a provider parameter
* here should be treated as a regression signal in review.
*/
export function getRequiredSetupScreens(options: {
theme: string | undefined
hasCompletedOnboarding: boolean | undefined
trustDialogAccepted: boolean
isClaubbit: boolean
}): { onboarding: boolean; trustDialog: boolean } {
return {
// Always show onboarding at least once (theme unset or never completed).
onboarding: !options.theme || !options.hasCompletedOnboarding,
// The trust dialog is the workspace trust boundary; only the claubbit
// harness (which owns its own trust story) skips it.
trustDialog: !options.isClaubbit && !options.trustDialogAccepted,
}
}