Skip to content

Delay onboarding web search until after file access#5738

Merged
kodjima33 merged 1 commit intomainfrom
codex/macos-onboarding-delay-search
Mar 17, 2026
Merged

Delay onboarding web search until after file access#5738
kodjima33 merged 1 commit intomainfrom
codex/macos-onboarding-delay-search

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • move onboarding web research until after file scan, task follow-up, and the background Gmail read attempt
  • add a Swift package test that locks the onboarding prompt ordering
  • allow custom desktop dev app names to carry matching bundle identifiers and document that rule in AGENTS/CLAUDE

Verification

  • cd desktop/Desktop && swift test
  • cd desktop/Desktop && swift build
  • built and launched /Applications/search.app as com.omi.search
  • runtime onboarding check in the built bundle confirmed file scan completed first, Gmail started next, and only then the post-scan "learning about you" phase appeared

@kodjima33 kodjima33 merged commit ce1a701 into main Mar 17, 2026
1 check passed
@kodjima33 kodjima33 deleted the codex/macos-onboarding-delay-search branch March 17, 2026 07:41
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 17, 2026

Greptile Summary

This PR reorders the desktop onboarding flow so that web research (previously STEP 2) is deferred to STEP 5 — after file scanning, task-candidate selection, and a background Gmail read attempt — giving the AI richer context before it searches the web. It also parameterises APP_NAME, BUNDLE_ID, and URL_SCHEME in run.sh to support custom-named test builds, adds developer-guidance prose in AGENTS.md/CLAUDE.md, and introduces a Swift test target that locks the step ordering as a regression gate.

Key changes:

  • ChatPrompts.swift: Web-research block moved from STEP 2 → STEP 5; all downstream step numbers (3-8) and the request_permission tool note updated correctly.
  • run.sh: APP_NAME, BUNDLE_ID, and URL_SCHEME are now env-var driven (OMI_APP_NAME, OMI_BUNDLE_ID, OMI_URL_SCHEME), enabling custom test builds without editing the script.
  • Package.swift + Tests/ChatPromptsTests.swift: New OmiComputerTests target with a single test that asserts STEP 2 < STEP 3 < STEP 4 < STEP 5 ordering in the built prompt string.
  • Issue: run.sh still unconditionally copies GoogleService-Info-Dev.plist (registered to com.omi.desktop-dev) for all dev builds, regardless of the active BUNDLE_ID. A custom bundle ID (e.g. com.omi.search) will receive a Firebase config whose bundle ID field doesn't match, potentially breaking Firebase Auth and Firestore initialisation.
  • Minor: The stale-bundle cleanup array in run.sh is hardcoded to Omi Dev.app and won't remove stale copies of a custom-named app.

Confidence Score: 3/5

  • Safe to merge for most use-cases, but the Firebase config mismatch in run.sh needs attention before custom-bundle-ID builds are used in practice.
  • The prompt reordering and test additions are clean and correct. The run.sh parameterisation is straightforward, but it introduces a silent breakage: GoogleService-Info-Dev.plist (bound to com.omi.desktop-dev) is copied unconditionally even when OMI_BUNDLE_ID is set to a different value, which can break Firebase Auth for custom test builds. The stale-app cleanup also doesn't cover custom names. These are real functional gaps for the stated use-case (custom-named test builds).
  • desktop/run.sh — GoogleService-Info-Dev.plist copy logic and CONFLICTING_APPS cleanup array both need updating to handle custom BUNDLE_ID / APP_NAME values.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/Chat/ChatPrompts.swift Moves web research from STEP 2 to STEP 5 (after file scan, task candidates, and background Gmail attempt); all step numbering references updated cleanly including the request_permission tool comment (Step 5 → Step 6).
desktop/Desktop/Tests/ChatPromptsTests.swift New regression test that verifies STEP 2 → STEP 3 → STEP 4 → STEP 5 ordering and that the web-research gate condition appears within STEP 5; correctly uses XCTUnwrap and String.Index comparisons.
desktop/Desktop/Package.swift Adds OmiComputerTests test target depending on the Omi Computer executable target; path resolves correctly to desktop/Desktop/Tests/.
desktop/run.sh Parameterises APP_NAME, BUNDLE_ID, and URL_SCHEME via env vars, but the GoogleService-Info-Dev.plist copy logic still unconditionally uses the plist registered for com.omi.desktop-dev regardless of the active BUNDLE_ID — a mismatch that can break Firebase when a custom bundle ID is used.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant O as Omi (LLM)
    participant FS as File System
    participant Gmail as Gmail (background)
    participant Web as Web Search

    O->>U: STEP 1 — Greet + confirm name
    O->>U: STEP 1.5 — Language preference
    O->>U: STEP 2 — Monthly goal
    U-->>O: Goal selected
    O->>FS: STEP 3 — scan_files()
    FS-->>O: Files, tools, frameworks found
    O->>Gmail: Background Gmail read attempt
    O->>U: STEP 4 — File discoveries + task candidates
    U-->>O: Task selected
    Note over O,Web: STEP 5 — Web research only AFTER file scan + Gmail attempt
    O->>Web: web_search(user_name + email_domain)
    Web-->>O: Company / role info
    O->>Web: web_search(company + product)
    Web-->>O: Project details
    O->>U: STEP 6 — Privacy note + permissions
    O->>U: STEP 7 — complete_onboarding()
    O->>U: STEP 8 — Deep dive
Loading

Comments Outside Diff (2)

  1. desktop/run.sh, line 246-251 (link)

    P1 GoogleService-Info-Dev.plist used regardless of custom BUNDLE_ID

    GoogleService-Info-Dev.plist was created for com.omi.desktop-dev. Now that BUNDLE_ID is driven by $OMI_BUNDLE_ID, a custom bundle ID (e.g. com.omi.search) causes a mismatch: Firebase is initialised with a plist whose BUNDLE_ID field is com.omi.desktop-dev, while the running app reports com.omi.search. This can break Firebase Auth (OAuth callback validation) and Firestore client initialisation.

    The copy step should fall back to the generic plist whenever the active bundle ID is not the default dev ID:

    substep "Copying GoogleService-Info.plist (dev version for $BUNDLE_ID)"
    if [ "$BUNDLE_ID" = "com.omi.desktop-dev" ] && [ -f "Desktop/Sources/GoogleService-Info-Dev.plist" ]; then
        cp -f Desktop/Sources/GoogleService-Info-Dev.plist "$APP_BUNDLE/Contents/Resources/GoogleService-Info.plist"
    else
        cp -f Desktop/Sources/GoogleService-Info.plist "$APP_BUNDLE/Contents/Resources/"
    fi

    Alternatively, add per-bundle-ID plist files (e.g. GoogleService-Info-search.plist) and select by $BUNDLE_ID.

  2. desktop/run.sh, line 92-112 (link)

    P2 Conflict-cleanup list hardcodes "Omi Dev" only

    CONFLICTING_APPS and the stale-bundle find still only target Omi Dev.app. When OMI_APP_NAME is set to a custom value (e.g. search), stale copies of search.app in ~/Downloads/ or ~/Desktop/ won't be removed on the next run, which can lead to LaunchServices confusion — the same class of problem the existing cleanup was meant to prevent.

    Consider extending the cleanup to also match $APP_NAME.app:

    CONFLICTING_APPS=(
        "/Applications/Omi Dev.app"
        "$HOME/Desktop/Omi Dev.app"
        "$HOME/Downloads/Omi Dev.app"
        "/Applications/$APP_NAME.app"
        "$HOME/Desktop/$APP_NAME.app"
        "$HOME/Downloads/$APP_NAME.app"
        ...
    )

Last reviewed commit: 95514c1

XCTAssertLessThan(step2Range.lowerBound, step3Range.lowerBound)
XCTAssertLessThan(step3Range.lowerBound, step4Range.lowerBound)
XCTAssertLessThan(step4Range.lowerBound, step5Range.lowerBound)
XCTAssertGreaterThan(gateRange.lowerBound, step5Range.lowerBound)
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 Gate assertion is trivially true relative to its purpose

XCTAssertGreaterThan(gateRange.lowerBound, step5Range.lowerBound) verifies that the gate sentence appears after the STEP 5 heading. While correct, this is trivially guaranteed by the structure of the prompt (the sentence lives inside STEP 5).

The stronger guarantee the test name promises — that web research is deferred until after file scan and task candidates — is already implied transitively through the chained XCTAssertLessThan calls. Consider adding an explicit assertion to document the intent more clearly:

// Gate must also appear after STEP 4 (file discoveries), making the constraint explicit.
XCTAssertGreaterThan(gateRange.lowerBound, step4Range.lowerBound)

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
## Summary
- move onboarding web research until after file scan, task follow-up,
and the background Gmail read attempt
- add a Swift package test that locks the onboarding prompt ordering
- allow custom desktop dev app names to carry matching bundle
identifiers and document that rule in AGENTS/CLAUDE

## Verification
- `cd desktop/Desktop && swift test`
- `cd desktop/Desktop && swift build`
- built and launched `/Applications/search.app` as `com.omi.search`
- runtime onboarding check in the built bundle confirmed file scan
completed first, Gmail started next, and only then the post-scan
"learning about you" phase appeared
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