-
Notifications
You must be signed in to change notification settings - Fork 8.9k
feat(onboarding): first-run experience for third-party providers #1864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
kevincodex1
merged 5 commits into
Gitlawb:main
from
kevincodex1:ui/third-party-onboarding
Jul 20, 2026
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
8a0b449
feat(onboarding): first-run experience for third-party providers
kevincodex1 1227275
fix(onboarding): address review — testable gating seam + env-profile …
kevincodex1 4096016
fix(onboarding): refresh reused profile credentials + accurate env va…
kevincodex1 2d23d56
fix(onboarding): preserve profile fields and verify the credential re…
kevincodex1 a00a29e
fix(onboarding): redact credential-bearing endpoints before display
kevincodex1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| }) | ||
| }) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, | ||
| } | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tests assert source-text shape, not actual runtime behavior.
Both new tests regex-match the raw text of
interactiveHelpers.tsxrather than exercisingshowSetupScreensitself. They'll pass as long as the literal stringusesAnthropicSetup && ...isn't present, regardless of whether the dialogs actually render correctly for third-party providers (or regress via an equivalent-but-differently-worded condition). As per path instructions, "Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior" — this change to startup dialog gating for all providers is exactly this kind of risky runtime change.Consider adding a behavioral test that mocks
usesAnthropicAccountFlow()to returnfalseandcheckHasTrustDialogAccepted()to returnfalse, then assertsshowSetupScreensactually invokes the onboarding/trust dialog imports/renderers for a non-Anthropic provider.🤖 Prompt for AI Agents
Source: Path instructions