mirror of
https://github.com/stablyai/orca.git
synced 2026-10-13 00:02:54 +00:00
fix(onboarding): suppress composer for folder repos and prune dead dismissed path
The composer is git-only (eligibleRepos filters with isGitRepoKind), so opening
it after a non-git first project leaves the user on a stuck modal with no
selectable repo. Gate the openModal call on isGit; folders complete onboarding
via activateAndRevealWorktree above.
Now that the only closeWith('dismissed', ...) caller is gone, narrow useCloseWith
to outcome:'completed', drop DismissedExtras and the unused lastStepReached
parameter, and remove the unreachable onboarding_dismissed track call.
Tighten the new e2e test: exact match for the absent Skip button, assert the
new repo-step subtitle, and poll lastCompletedStep so the prior Skip's IPC
flush doesn't race the snapshot.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -3,7 +3,7 @@ import { track } from '@/lib/telemetry'
|
||||
import { ONBOARDING_FINAL_STEP } from '../../../../shared/constants'
|
||||
import type { GlobalSettings, OnboardingState, TuiAgent } from '../../../../shared/types'
|
||||
import type { NotificationDraft } from './NotificationStep'
|
||||
import type { StepId, StepNumber } from './use-onboarding-flow-types'
|
||||
import type { StepId } from './use-onboarding-flow-types'
|
||||
|
||||
export async function persistStep(
|
||||
stepNumber: number,
|
||||
@@ -26,11 +26,6 @@ type CloseWithDeps = {
|
||||
setError: (msg: string | null) => void
|
||||
}
|
||||
|
||||
export type DismissedExtras = {
|
||||
advancedVia: 'button' | 'keyboard'
|
||||
durationMs: number
|
||||
}
|
||||
|
||||
export function useCloseWith({
|
||||
onOnboardingChange,
|
||||
onboardingChecklist,
|
||||
@@ -39,24 +34,23 @@ export function useCloseWith({
|
||||
}: CloseWithDeps) {
|
||||
return useCallback(
|
||||
async (
|
||||
outcome: 'completed' | 'dismissed',
|
||||
outcome: 'completed',
|
||||
checklist: Partial<OnboardingState['checklist']>,
|
||||
lastStepReached: StepNumber,
|
||||
completedPath?: 'open_folder' | 'clone_url',
|
||||
dismissedExtras?: DismissedExtras
|
||||
completedPath?: 'open_folder' | 'clone_url'
|
||||
): Promise<boolean> => {
|
||||
let nextState: OnboardingState
|
||||
try {
|
||||
// Why: main-process updateOnboarding already merges with current state,
|
||||
// so spreading the local (potentially stale) onboarding.checklist would
|
||||
// overwrite concurrent updates.
|
||||
// overwrite concurrent updates. Writing dismissed:false explicitly
|
||||
// resets any prior on-disk true value via the merge.
|
||||
nextState = await window.api.onboarding.update({
|
||||
closedAt: Date.now(),
|
||||
outcome,
|
||||
lastCompletedStep: outcome === 'completed' ? ONBOARDING_FINAL_STEP : -1,
|
||||
lastCompletedStep: ONBOARDING_FINAL_STEP,
|
||||
checklist: {
|
||||
...checklist,
|
||||
dismissed: outcome === 'dismissed'
|
||||
dismissed: false
|
||||
}
|
||||
})
|
||||
} catch (err) {
|
||||
@@ -64,7 +58,7 @@ export function useCloseWith({
|
||||
return false
|
||||
}
|
||||
onOnboardingChange(nextState)
|
||||
if (outcome === 'completed' && completedPath) {
|
||||
if (completedPath) {
|
||||
const total = Math.max(0, Date.now() - startTimeRef.current)
|
||||
track('onboarding_completed', {
|
||||
path: completedPath,
|
||||
@@ -87,16 +81,6 @@ export function useCloseWith({
|
||||
time_since_completed_ms: 0
|
||||
})
|
||||
}
|
||||
} else if (outcome === 'dismissed') {
|
||||
track('onboarding_dismissed', {
|
||||
last_step: lastStepReached,
|
||||
...(dismissedExtras
|
||||
? {
|
||||
duration_ms: dismissedExtras.durationMs,
|
||||
advanced_via: dismissedExtras.advancedVia
|
||||
}
|
||||
: {})
|
||||
})
|
||||
}
|
||||
return true
|
||||
},
|
||||
|
||||
@@ -252,7 +252,6 @@ export function useOnboardingFlow(
|
||||
const closed = await closeWith(
|
||||
'completed',
|
||||
isGit ? { addedRepo: true } : { addedFolder: true },
|
||||
4,
|
||||
path
|
||||
)
|
||||
if (!closed) {
|
||||
@@ -267,13 +266,19 @@ export function useOnboardingFlow(
|
||||
value_kind: 'repo',
|
||||
duration_ms: consumeStepDurationMs()
|
||||
})
|
||||
// Why: plain folders are valid first projects too; suppressing the
|
||||
// composer for them strands users after onboarding with no workspace.
|
||||
openModal('new-workspace-composer', {
|
||||
initialRepoId: repoId,
|
||||
prefilledName: 'onboarding',
|
||||
telemetrySource: 'onboarding'
|
||||
})
|
||||
// Why: plain folders are valid first projects, but the composer is
|
||||
// git-only (useComposerState filters to git repos and creates worktrees,
|
||||
// which non-git folders can't do). Folders complete onboarding via the
|
||||
// activateAndRevealWorktree call above, which navigates the user
|
||||
// straight into the folder workspace; opening the composer for them
|
||||
// would land them on a stuck modal with no selectable repo.
|
||||
if (isGit) {
|
||||
openModal('new-workspace-composer', {
|
||||
initialRepoId: repoId,
|
||||
prefilledName: 'onboarding',
|
||||
telemetrySource: 'onboarding'
|
||||
})
|
||||
}
|
||||
},
|
||||
[closeWith, consumeStepDurationMs, fetchRepos, fetchWorktrees, openModal]
|
||||
)
|
||||
@@ -398,7 +403,7 @@ export function useOnboardingFlow(
|
||||
}
|
||||
// Why: skip has no keyboard path today, so `advanced_via` is always
|
||||
// `'button'`. Including the field keeps the shape uniform with the
|
||||
// completed/dismissed events and lets a future keyboard-skip arrive
|
||||
// completed events and lets a future keyboard-skip arrive
|
||||
// without a schema migration.
|
||||
const durationMs = consumeStepDurationMs()
|
||||
track('onboarding_step_skipped', {
|
||||
|
||||
@@ -333,13 +333,25 @@ test.describe('Onboarding flow', () => {
|
||||
).toBeVisible()
|
||||
await orcaPage.getByRole('button', { name: 'Skip' }).click()
|
||||
await expect(orcaPage.getByRole('heading', { name: /Add your first project/i })).toBeVisible()
|
||||
await expect(
|
||||
orcaPage.getByText(/Orca needs a folder or repo before it can create a workspace/i)
|
||||
).toBeVisible()
|
||||
await expect(orcaPage.getByRole('button', { name: /I'll add one later/ })).toHaveCount(0)
|
||||
await expect(orcaPage.getByRole('button', { name: 'Skip' })).toHaveCount(0)
|
||||
// Why: exact match — any unrelated "Skip…" button could give a false
|
||||
// negative (mirrors the 'Back' exact-match rationale above).
|
||||
await expect(orcaPage.getByRole('button', { name: 'Skip', exact: true })).toHaveCount(0)
|
||||
|
||||
// Why: the previous Skip click flushes persistence async via IPC, so poll
|
||||
// for lastCompletedStep === 3 before snapshotting the rest of the state.
|
||||
await expect
|
||||
.poll(async () => (await getOnboardingState(orcaPage)).lastCompletedStep, {
|
||||
timeout: 5_000
|
||||
})
|
||||
.toBe(3)
|
||||
|
||||
const final = await getOnboardingState(orcaPage)
|
||||
expect(final.closedAt).toBeNull()
|
||||
expect(final.outcome).toBeNull()
|
||||
expect(final.checklist.dismissed).toBe(false)
|
||||
expect(final.lastCompletedStep).toBe(3)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user