Skip to content

Fix desktop onboarding reset scoping #5604

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.

Sign up for GitHub

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
kodjima33 merged 1 commit into from
Mar 13, 2026
+24 −2
Merged

Fix desktop onboarding reset scoping #5604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions desktop/Desktop/Sources/AppState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2466,6 +2466,12 @@ class AppState: ObservableObject {
nonisolated func resetOnboardingAndRestart() {
log("Resetting onboarding state for current app...")

// Update live @AppStorage state in the current app instance before touching
// raw UserDefaults so SwiftUI doesn't write stale onboarding values back.
DispatchQueue.main.async {
NotificationCenter.default.post(name: .resetOnboardingRequested, object: nil)
}
Comment on lines +2469 to +2473
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 13, 2026

Choose a reason for hiding this comment

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

Misleading comment — notification fires after UserDefaults clearing on the main thread

The inline comment says "before touching raw UserDefaults", but both call sites (SettingsPage.swift button action and OmiApp.swift's @MainActor resetOnboarding()) invoke this function on the main thread. Because DispatchQueue.main.async simply enqueues a block for the next run-loop iteration, the synchronous UserDefaults.standard.removeObject loop below runs first. The notification is therefore dispatched after the keys are already cleared, not before.

This doesn't cause a functional bug in practice (clearing + writing 0/false produces the same final state), but the comment misrepresents the ordering and could mislead future readers about the intent of the guard.

Consider either:

  1. Posting the notification synchronously (before the removal loop) to match the comment's intent:
// Reset live @AppStorage before clearing raw UserDefaults
NotificationCenter.default.post(name: .resetOnboardingRequested, object: nil)

// Clear onboarding-related UserDefaults keys
for key in onboardingKeys { ... }
  1. Or, keeping the current order and correcting the comment to reflect what actually happens:
// Clear raw UserDefaults first, then reset live @AppStorage via notification
// so any subsequent @AppStorage write-backs use 0/false (the desired reset state).
DispatchQueue.main.async {
    NotificationCenter.default.post(name: .resetOnboardingRequested, object: nil)
}


// Clear onboarding-related UserDefaults keys (thread-safe, do first)
let onboardingKeys = [
"hasCompletedOnboarding",
Expand Down Expand Up @@ -2503,6 +2509,7 @@ class AppState: ObservableObject {

// Restart off the main thread to avoid blocking the menu action path.
DispatchQueue.global(qos: .utility).async { [self] in
Thread.sleep(forTimeInterval: 0.15)
Comment on lines 2511 to +2512
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 13, 2026

Choose a reason for hiding this comment

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

Thread.sleep blocks a GCD thread pool worker

Thread.sleep(forTimeInterval: 0.15) ties up a thread in the shared GCD utility thread pool for 150 ms. While this runs just before termination and is unlikely to cause starvation in practice, it is an unconventional pattern when DispatchQueue.main.asyncAfter (or a Task { try await Task.sleep(...) }) would express the delay without blocking a pool thread.

DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { [self] in
    self.restartApp()
}

This also removes the need for DispatchQueue.global(qos: .utility).async wrapping.

// Keep onboarding reset scoped to the current app instance.
// It must not mutate production defaults, shared local data, or TCC permissions.
self.restartApp()
Expand Down Expand Up @@ -2773,6 +2780,8 @@ class AppState: ObservableObject {
// MARK: - System Event Notification Names

extension Notification.Name {
/// Posted when the current app instance should fully clear its own onboarding state.
static let resetOnboardingRequested = Notification.Name("resetOnboardingRequested")
/// Posted when the system wakes from sleep
static let systemDidWake = Notification.Name("systemDidWake")
/// Posted when the screen is locked
Expand Down
9 changes: 9 additions & 0 deletions desktop/Desktop/Sources/MainWindow/DesktopHomeView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ struct DesktopHomeView: View {
}()
@State private var isSidebarCollapsed: Bool = false
@AppStorage("currentTierLevel") private var currentTierLevel = 0
@AppStorage("onboardingStep") private var onboardingStep = 0
@AppStorage("onboardingJustCompleted") private var onboardingJustCompleted = false

// Settings sidebar state
@State private var selectedSettingsSection: SettingsContentView.SettingsSection = .general
Expand Down Expand Up @@ -201,6 +203,13 @@ struct DesktopHomeView: View {
appState.hasCompletedOnboarding = false
appState.stopTranscription()
}
.onReceive(NotificationCenter.default.publisher(for: .resetOnboardingRequested)) { _ in
log("DesktopHomeView: resetOnboardingRequested — clearing live onboarding state for current app")
appState.hasCompletedOnboarding = false
onboardingStep = 0
onboardingJustCompleted = false
appState.stopTranscription()
}
Comment on lines +206 to +212
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 13, 2026

Choose a reason for hiding this comment

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

@AppStorage writes recreate the UserDefaults keys that were just removed

resetOnboardingAndRestart() removes "onboardingStep" and "onboardingJustCompleted" from UserDefaults synchronously. On the very next run-loop iteration, this handler fires and the @AppStorage setter assignments immediately write those same keys back into UserDefaults with their default values (0 / false).

The net result is functionally identical to having removed them — both paths produce the default value on next read. However, it is worth noting that after the notification fires the keys will be present in UserDefaults (value 0/false) rather than absent. If the intent is strictly "keys absent after reset," the two explicit @AppStorage writes here are unnecessary since @AppStorage returns the declared default automatically when a key is missing.

// Handle transcription toggle from menu bar
.onReceive(NotificationCenter.default.publisher(for: .toggleTranscriptionRequested)) { notification in
if let enabled = notification.userInfo?["enabled"] as? Bool {
Expand Down
4 changes: 2 additions & 2 deletions desktop/Desktop/Sources/MainWindow/Pages/SettingsPage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3801,7 +3801,7 @@ struct SettingsContentView: View {
.scaledFont(size: 16, weight: .semibold)
.foregroundColor(OmiColors.textPrimary)

Text("Restart setup wizard and reset permissions")
Text("Restart setup wizard for this app build only")
.scaledFont(size: 13)
.foregroundColor(OmiColors.textTertiary)
}
Expand All @@ -3828,7 +3828,7 @@ struct SettingsContentView: View {
appState.resetOnboardingAndRestart()
}
} message: {
Text("This will reset all permissions, clear chat history, and restart the app. You'll need to grant permissions again during setup.")
Text("This will reset onboarding for this app build only, clear onboarding chat history, and restart the app without affecting the other installed build.")
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions desktop/Desktop/Sources/OnboardingView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ struct OnboardingView: View {
// Pre-warm the ACP bridge before the chat step starts.
await chatProvider.warmupBridge()
}
.onReceive(NotificationCenter.default.publisher(for: .resetOnboardingRequested)) { _ in
log("OnboardingView: resetOnboardingRequested — returning to chat step for current app")
currentStep = 0
}
}

private var onboardingContent: some View {
Expand Down