Skip to content

Fix onboarding: reorder flow, fix restart recovery, fix keychain prompts#5973

Merged
kodjima33 merged 5 commits into
mainfrom
worktree-fix-onboarding-restart-recovery
Mar 24, 2026
Merged

Fix onboarding: reorder flow, fix restart recovery, fix keychain prompts#5973
kodjima33 merged 5 commits into
mainfrom
worktree-fix-onboarding-restart-recovery

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • Keychain fix: Replace security CLI with SecItemCopyMatching for browser cookie access — "Always Allow" now persists across launches. Remove Brave/Edge/Vivaldi, only check Arc + Chrome.
  • Onboarding reorder: Move goal question AFTER all permissions so app restarts never skip it. New flow: scan + email → non-restart permissions → web research → screen recording → email insights + goal → complete.
  • get_email_insights tool: New onboarding tool surfaces Gmail/Calendar background reading results to the AI so goal suggestions are informed by email context.
  • Restart recovery: Now trivial — after any restart, only email insights + goal + complete remain.

Test plan

  • Fresh onboarding: verify goal question appears after all permissions
  • Verify no repeated keychain password prompts (only Arc/Chrome attempted)
  • Grant Screen Recording → app restarts → verify goal is asked after restart
  • Verify email insights are surfaced if Gmail session available

🤖 Generated with Claude Code

kodjima33 and others added 5 commits March 23, 2026 23:52
…move Brave/Edge/Vivaldi

Replace security CLI with native Security framework for cookie decryption
passwords. This ties "Always Allow" to Omi's code signature so it persists
across launches. Also removes Brave, Edge, and Vivaldi from the browser
list — only Arc and Chrome are checked now.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…emove Brave/Edge/Vivaldi

Mirror the GmailReaderService keychain fix for CalendarReaderService.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…estart recovery

Move goal question after all permissions so app restarts never skip it.
New flow: scan + email → non-restart permissions → web research →
screen recording → email insights + goal → complete.
Restart recovery is now trivial: only goal + complete remain after restart.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New tool surfaces Gmail/Calendar synthesis results to the onboarding
chat AI so it can inform goal suggestions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Set ChatToolExecutor.emailInsightsText and calendarInsightsText when
background reading completes so the onboarding AI can access them.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kodjima33 kodjima33 merged commit d90013b into main Mar 24, 2026
3 checks passed
@kodjima33 kodjima33 deleted the worktree-fix-onboarding-restart-recovery branch March 24, 2026 03:52
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR improves the macOS desktop onboarding experience through three focused changes: replacing the security CLI with the native SecItemCopyMatching API for persistent keychain access, reordering the onboarding flow so the goal question comes after all permissions (preventing post-restart step loss), and introducing a new get_email_insights tool that surfaces Gmail/Calendar background-reading results to the AI for more informed goal suggestions.

Key changes:

  • GmailReaderService and CalendarReaderService: getKeychainPassword now calls SecItemCopyMatching directly instead of spawning a security subprocess — "Always Allow" in the keychain dialog now persists to the app itself, not to the external tool. Brave, Edge, and Vivaldi configs removed; only Arc and Chrome remain.
  • ChatPrompts.swift: Onboarding prompt refactored from 6 to 8 steps. Screen Recording moved to Step 5 (last permission), goal question moved to Step 6 after email insights. Restart recovery simplified: only Step 6 (insights + goal) + Step 7 (complete) remain post-restart.
  • ChatToolExecutor.swift: New get_email_insights tool reads two new static vars (emailInsightsText, calendarInsightsText) populated by OnboardingChatView when background reading completes.
  • One bug found: The get_email_insights tool documentation in the prompt says "Call this in Step 4" and "inform goal suggestions in Step 5", but Step 4 is WEB RESEARCH and Step 5 is SCREEN RECORDING — the tool is actually used in Step 6. This stale reference can cause the LLM to call the tool out of order.

Confidence Score: 4/5

  • Safe to merge after fixing the step reference bug in the get_email_insights tool docs — everything else is solid.
  • The keychain and onboarding reorder changes are clean and well-reasoned. The single concrete issue is the stale step numbers in the LLM tool documentation ("Step 4"/"Step 5" vs the correct "Step 6"), which could cause the AI to invoke get_email_insights during web research instead of after screen recording. This is a one-line fix and doesn't affect any runtime code. The restart-recovery race condition is a graceful degradation (returns "no data" rather than crashing), keeping the score at 4 rather than lower.
  • desktop/Desktop/Sources/Chat/ChatPrompts.swift — fix the two stale step references in the get_email_insights tool description (lines 787–788).

Important Files Changed

Filename Overview
desktop/Desktop/Sources/Chat/ChatPrompts.swift Onboarding prompt reworked into 8 steps. Contains a critical step-numbering mismatch in the get_email_insights tool docs (says "Step 4"/"Step 5" but the tool is used in Step 6), which could cause the LLM to invoke the tool out of order.
desktop/Desktop/Sources/Providers/ChatToolExecutor.swift Adds get_email_insights tool backed by two new static vars (emailInsightsText, calendarInsightsText). Implementation is correct and thread-safe via @mainactor. Static vars are nil after restart, creating a potential race with the background email reading.
desktop/Desktop/Sources/GmailReaderService.swift Replaces the security CLI subprocess with SecItemCopyMatching for keychain access. Correct approach — "Always Allow" now binds to the app process. Removes Brave/Edge/Vivaldi, keeping only Arc and Chrome.
desktop/Desktop/Sources/CalendarReaderService.swift Mirrors the GmailReaderService keychain change — same SecItemCopyMatching pattern, same browser list reduction. Clean and consistent.
desktop/Desktop/Sources/OnboardingChatView.swift Wires email and calendar synthesis results into ChatToolExecutor static vars so they're accessible to the new get_email_insights tool. Minimal, correct change.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant AI as Onboarding AI
    participant App as Desktop App
    participant KC as Keychain (SecItemCopyMatching)
    participant BG as Background Tasks

    U->>AI: Step 1 — Name + Language
    AI->>App: set_user_preferences + save_knowledge_graph

    AI->>App: Step 2 — scan_files (BLOCKING)
    App->>BG: Start Gmail reading (async)
    App->>BG: Start Calendar reading (async)
    AI->>App: save_knowledge_graph (file findings)

    AI->>App: Step 3 — check_permission_status
    App-->>AI: permission statuses
    AI->>App: request_permission(microphone → notifications → accessibility → automation)

    AI->>U: Step 4 — Web research (3 searches)
    AI->>App: save_knowledge_graph (company/projects)

    AI->>App: Step 5 — request_permission(screen_recording)
    Note over App: App may restart here

    alt App restarts
        App->>BG: Re-start Gmail + Calendar reading
        AI->>App: get_email_insights (may return "in progress")
    else No restart
        BG-->>App: emailInsightsText set
        BG-->>App: calendarInsightsText set
        AI->>App: Step 6 — get_email_insights
        App-->>AI: Email + Calendar summaries
    end

    AI->>U: Step 6 — ask_followup (monthly goal)
    U-->>AI: Goal selected
    AI->>App: save_knowledge_graph (goal node)

    AI->>App: Step 7 — complete_onboarding
    App-->>U: Continue to App

    Note over KC: Keychain access now uses<br/>SecItemCopyMatching directly<br/>(persistent "Always Allow" per app)
Loading

Reviews (1): Last reviewed commit: "fix(desktop): bridge Gmail/Calendar resu..." | Re-trigger Greptile

Comment on lines +787 to +788
- Call this in Step 4. The background reading starts automatically after file scan — by the time you reach Step 4, it's usually done.
- Use the results to inform goal and task suggestions in Step 5.
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.

P1 Stale step references in tool docs will mislead the LLM

The get_email_insights tool description says "Call this in Step 4" and "Use the results to inform goal and task suggestions in Step 5", but the actual prompt structure places get_email_insights in Step 6 and the monthly goal is also set in Step 6 (not Step 5, which is now Screen Recording). Step 4 is WEB RESEARCH, and Step 5 is SCREEN RECORDING.

The LLM reads the tool docs as a reference guide. If the docs say to call this tool in Step 4 (WEB RESEARCH), the model may invoke get_email_insights during web research instead of waiting for Step 6. This creates conflicting instructions — the step-by-step body says one thing, the tool docs say another.

Suggested change
- Call this in Step 4. The background reading starts automatically after file scan — by the time you reach Step 4, it's usually done.
- Use the results to inform goal and task suggestions in Step 5.
- Call this in Step 6. The background reading starts automatically after file scan — by the time you reach Step 6, it's usually done.
- Use the results to inform goal and task suggestions in Step 6.

Comment on lines 828 to 851
}
}

/// Return email/calendar insights from background reading
private static func executeGetEmailInsights() -> String {
var sections: [String] = []

if let email = emailInsightsText, !email.isEmpty {
sections.append("## Email Insights\n\(email)")
}
if let calendar = calendarInsightsText, !calendar.isEmpty {
sections.append("## Calendar Insights\n\(calendar)")
}

if sections.isEmpty {
return "No email insights available yet. The background reading may still be in progress, or no browser with a Gmail session was found."
}

return sections.joined(separator: "\n\n")
}

/// Set user preferences (language, name)
private static func executeSetUserPreferences(_ args: [String: Any]) async -> String {
var results: [String] = []
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.

P2 get_email_insights may always return "no data" after restart

After an app restart, emailInsightsText and calendarInsightsText are reset to nil (static vars). In OnboardingChatView, startGmailReading() / startCalendarReading() are only triggered once files are found indexed (line ~1331). The RESTART RECOVERY prompt instructs the AI to call get_email_insights as its very first tool call, but background reading is async and may not have completed by then — causing the tool to consistently return "No email insights available yet" immediately after restart.

Since the restart scenario is one of the key improvements this PR advertises, consider adding a short wait/poll or returning a distinct signal (e.g. "Email reading in progress — try again shortly.") so the LLM can optionally retry after the user finishes typing their goal response, rather than silently skipping insights.

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…pts (BasedHardware#5973)

## Summary
- **Keychain fix**: Replace `security` CLI with `SecItemCopyMatching`
for browser cookie access — "Always Allow" now persists across launches.
Remove Brave/Edge/Vivaldi, only check Arc + Chrome.
- **Onboarding reorder**: Move goal question AFTER all permissions so
app restarts never skip it. New flow: scan + email → non-restart
permissions → web research → screen recording → email insights + goal →
complete.
- **`get_email_insights` tool**: New onboarding tool surfaces
Gmail/Calendar background reading results to the AI so goal suggestions
are informed by email context.
- **Restart recovery**: Now trivial — after any restart, only email
insights + goal + complete remain.

## Test plan
- [ ] Fresh onboarding: verify goal question appears after all
permissions
- [ ] Verify no repeated keychain password prompts (only Arc/Chrome
attempted)
- [ ] Grant Screen Recording → app restarts → verify goal is asked after
restart
- [ ] Verify email insights are surfaced if Gmail session available

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant