Skip to content
63 changes: 63 additions & 0 deletions code/core/src/core-server/build-dev.onboarding.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';

// Use vi.hoisted so these are available when vi.mock factory runs
const { mockCacheGet, mockCacheRemove, mockDetectAgent } = vi.hoisted(() => ({
mockCacheGet: vi.fn(),
mockCacheRemove: vi.fn(),
mockDetectAgent: vi.fn(),
}));

vi.mock('storybook/internal/common', async (importOriginal) => {
const actual = await importOriginal<typeof import('storybook/internal/common')>();
return {
...actual,
cache: {
get: mockCacheGet,
set: vi.fn(),
remove: mockCacheRemove,
},
};
});

vi.mock('storybook/internal/telemetry', async (importOriginal) => {
const actual = await importOriginal<typeof import('storybook/internal/telemetry')>();
return { ...actual, detectAgent: mockDetectAgent };
});

import { resolveOnboardingInitialPath } from './build-dev.ts';

describe('resolveOnboardingInitialPath', () => {
beforeEach(() => {
vi.clearAllMocks();
mockDetectAgent.mockReturnValue(undefined); // default: not an agent
});

it('returns /onboarding and removes cache entry when onboarding-pending is set and no CLI initialPath', async () => {
mockCacheGet.mockResolvedValue(true);
const result = await resolveOnboardingInitialPath(undefined);
expect(result).toBe('/onboarding');
expect(mockCacheRemove).toHaveBeenCalledWith('onboarding-pending');
});

it('returns undefined and does not remove cache when onboarding-pending is absent', async () => {
mockCacheGet.mockResolvedValue(undefined);
const result = await resolveOnboardingInitialPath(undefined);
expect(result).toBeUndefined();
expect(mockCacheRemove).not.toHaveBeenCalled();
});

it('returns CLI initialPath and does NOT remove cache when CLI initialPath is already set', async () => {
mockCacheGet.mockResolvedValue(true);
const result = await resolveOnboardingInitialPath('/my-story');
expect(result).toBe('/my-story');
expect(mockCacheRemove).not.toHaveBeenCalled();
});

it('returns undefined and does NOT remove cache when running in an agent context', async () => {
mockDetectAgent.mockReturnValue({ name: 'claude' });
mockCacheGet.mockResolvedValue(true);
const result = await resolveOnboardingInitialPath(undefined);
expect(result).toBeUndefined();
expect(mockCacheRemove).not.toHaveBeenCalled();
});
});
48 changes: 41 additions & 7 deletions code/core/src/core-server/build-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { readFile } from 'node:fs/promises';

import {
JsPackageManagerFactory,
cache,
getConfigInfo,
getInterpretedFile,
getProjectRoot,
Expand All @@ -15,7 +16,12 @@ import {
} from 'storybook/internal/common';
import { CLI_COLORS, deprecate, logger, prompt } from 'storybook/internal/node-logger';
import { MissingBuilderError, NoStatsForViteDevError } from 'storybook/internal/server-errors';
import { oneWayHash, setTelemetryEnabled, telemetry } from 'storybook/internal/telemetry';
import {
detectAgent,
oneWayHash,
setTelemetryEnabled,
telemetry,
} from 'storybook/internal/telemetry';
import type { BuilderOptions, CLIOptions, LoadOptions, Options } from 'storybook/internal/types';

import { global } from '@storybook/global';
Expand All @@ -41,6 +47,30 @@ import { updateCheck } from './utils/update-check.ts';
import { warnOnIncompatibleAddons } from './utils/warnOnIncompatibleAddons.ts';
import { warnWhenUsingArgTypesRegex } from './utils/warnWhenUsingArgTypesRegex.ts';

/**
* Resolves the initialPath for the browser open URL.
* CLI-provided initialPath always wins. If not set and not running in an agent context,
* checks the project cache for an `onboarding-pending` entry written by `storybook init`.
* If found, returns '/onboarding' and removes the cache entry so it only triggers once.
* The cache entry is only written by init when onboarding is known to be supported,
* so no further addon check is needed here.
*/
export async function resolveOnboardingInitialPath(
cliInitialPath: string | undefined
): Promise<string | undefined> {
if (cliInitialPath || detectAgent()) {
// Explicit CLI flag wins; leave cache intact for next run.
// Agent environments skip onboarding (no browser to open).
return cliInitialPath;
}
const onboardingPending = await cache.get('onboarding-pending').catch(() => {});
if (onboardingPending) {
await cache.remove('onboarding-pending').catch(() => {});
return '/onboarding';
}
return undefined;
}

export async function buildDevStandalone(
options: CLIOptions &
LoadOptions &
Expand Down Expand Up @@ -87,20 +117,23 @@ export async function buildDevStandalone(

const cacheKey = oneWayHash(relative(getProjectRoot(), configDir));

const cacheOutputDir = resolvePathInStorybookCache('public', cacheKey);
let outputDir = resolve(options.outputDir || cacheOutputDir);
if (options.smokeTest) {
outputDir = cacheOutputDir;
}

// Resolve initialPath: CLI flag takes precedence; fall back to onboarding-pending cache entry.
invariant(port, 'expected options to have a port');
options.initialPath = await resolveOnboardingInitialPath(options.initialPath);

const { address: localAddress, networkAddress } = getServerAddresses(
port,
options.host,
options.https ? 'https' : 'http',
options.initialPath
);

const cacheOutputDir = resolvePathInStorybookCache('public', cacheKey);
let outputDir = resolve(options.outputDir || cacheOutputDir);
if (options.smokeTest) {
outputDir = cacheOutputDir;
}

options.port = port;
options.versionCheck = versionCheck;
options.configType = 'DEVELOPMENT';
Expand All @@ -124,6 +157,7 @@ export async function buildDevStandalone(

const config = await loadMainConfig(options);
const { core, framework } = config;

const corePresets = [];

let frameworkName = typeof framework === 'string' ? framework : framework?.name;
Expand Down
16 changes: 8 additions & 8 deletions code/lib/create-storybook/src/initiate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ProjectType } from 'storybook/internal/cli';
import {
type JsPackageManager,
PackageManagerName,
cache,
executeCommand,
} from 'storybook/internal/common';
import { getServerPort, withTelemetry } from 'storybook/internal/core-server';
Expand Down Expand Up @@ -158,6 +159,12 @@ export async function doInitiate(options: CommandOptions): Promise<
// Step 9: Track telemetry
await telemetryService.trackInitWithContext(projectType, selectedFeatures, newUser);

// Signal dev to redirect to onboarding on first run
const shouldOnboard = newUser;
if (shouldOnboard && FeatureCompatibilityService.supportsOnboarding(projectType)) {
await cache.set('onboarding-pending', true).catch(() => {});
}
Comment on lines +164 to +166

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make onboarding cache persistence best-effort so init cannot fail after successful setup.

A failure in cache.set(...) currently bubbles up and can fail init, even though onboarding redirection is non-critical state.

Proposed fix
   const shouldOnboard = newUser;
   if (shouldOnboard && FeatureCompatibilityService.supportsOnboarding(projectType)) {
-    await cache.set('onboarding-pending', true);
+    try {
+      await cache.set('onboarding-pending', true);
+    } catch (error) {
+      logger.debug(
+        `Failed to persist onboarding state: ${
+          error instanceof Error ? error.message : String(error)
+        }`
+      );
+    }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/lib/create-storybook/src/initiate.ts` around lines 164 - 166, The
onboarding cache write (cache.set('onboarding-pending', true)) can throw and
currently lets init fail; make this persistence best-effort by wrapping the
cache.set call used when shouldOnboard &&
FeatureCompatibilityService.supportsOnboarding(projectType) in a try/catch
inside the init flow (or the function that contains these symbols), await the
set as before but catch and log the error (or debug) without rethrowing so init
proceeds even if cache persistence fails.


return {
shouldRunDev:
!!options.dev &&
Expand Down Expand Up @@ -213,17 +220,14 @@ async function runStorybookDev(result: {
projectType: ProjectType;
packageManager: JsPackageManager;
storybookCommand?: string | null;
shouldOnboard: boolean;
}): Promise<void> {
const { projectType, packageManager, storybookCommand, shouldOnboard } = result;
const { projectType, packageManager, storybookCommand } = result;

if (!storybookCommand) {
return;
}

try {
const supportsOnboarding = FeatureCompatibilityService.supportsOnboarding(projectType);

const parts = storybookCommand.split(' ');

// Angular CLI throws "Unknown argument: silent"
Expand Down Expand Up @@ -252,10 +256,6 @@ async function runStorybookDev(result: {
parts.push(`-p`, `${availablePort}`);
}

if (supportsOnboarding && shouldOnboard) {
parts.push('--initial-path=/onboarding');
}

parts.push('--quiet');
}

Expand Down
Loading