Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion app/src/pages/onboarding/OnboardingLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,26 @@ const OnboardingLayout = () => {
});

try {
// Preserve a tool preference the user already customized (e.g. via
// Settings → Tools or an earlier onboarding run) rather than resetting
// to catalog defaults on every completion. Re-applying defaults here
// could silently narrow an existing selection. Only seed defaults when
// no preference has been persisted yet. The Rust-side filter
// (`filter_tools_by_user_preference`) is the authoritative guard against
// stale snapshots stripping newer tools (issue #3096); this is
// defense-in-depth on the write path.
const existingEnabledTools = snapshot.localState.onboardingTasks?.enabledTools;
const enabledTools =
existingEnabledTools && existingEnabledTools.length > 0
? existingEnabledTools
: getEnabledRustToolNames(getDefaultEnabledTools());

await setOnboardingTasks({
accessibilityPermissionGranted:
snapshot.localState.onboardingTasks?.accessibilityPermissionGranted ?? false,
localModelConsentGiven: false,
localModelDownloadStarted: false,
enabledTools: getEnabledRustToolNames(getDefaultEnabledTools()),
enabledTools,
connectedSources: draft.connectedSources,
updatedAtMs: Date.now(),
});
Expand Down
40 changes: 38 additions & 2 deletions app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ function buildStore() {
});
}

async function setupLayout() {
async function setupLayout(onboardingTasks: unknown = null) {
const { useCoreState } = await import('../../../providers/CoreStateProvider');

const mockSetOnboardingCompletedFlag = vi.fn().mockResolvedValue(undefined);
Expand All @@ -114,7 +114,7 @@ async function setupLayout() {
onboardingCompleted: false,
chatOnboardingCompleted: false,
analyticsEnabled: false,
localState: { encryptionKey: null, onboardingTasks: null, keyringConsent: null },
localState: { encryptionKey: null, onboardingTasks, keyringConsent: null },
keyringStatus: {
available: true,
failureReason: null,
Expand Down Expand Up @@ -254,4 +254,40 @@ describe('OnboardingLayout — Joyride walkthrough integration (#1123)', () => {

warnSpy.mockRestore();
});

// [#3096] Tool preference persistence on completion.
it('seeds default enabledTools when no preference was persisted', async () => {
const { mockSetOnboardingTasks } = await setupLayout(null);

await act(async () => {
fireEvent.click(screen.getByTestId('complete-btn'));
});

// Mocks: getDefaultEnabledTools() → [], getEnabledRustToolNames(x) → x.
expect(mockSetOnboardingTasks).toHaveBeenCalledWith(
expect.objectContaining({ enabledTools: [] })
);
});

it('preserves an existing customized enabledTools list instead of resetting to defaults', async () => {
const existing = ['shell', 'cron_add', 'cron_list'];
const { mockSetOnboardingTasks } = await setupLayout({
accessibilityPermissionGranted: false,
localModelConsentGiven: false,
localModelDownloadStarted: false,
enabledTools: existing,
connectedSources: [],
updatedAtMs: 1,
});

await act(async () => {
fireEvent.click(screen.getByTestId('complete-btn'));
});

// The user's prior selection must be carried through verbatim — completing
// onboarding must never silently narrow an already-customized tool list.
expect(mockSetOnboardingTasks).toHaveBeenCalledWith(
expect.objectContaining({ enabledTools: existing })
);
});
});
Loading
Loading