Skip to content

Fix desktop test build identity and onboarding recovery#5778

Merged
kodjima33 merged 1 commit intomainfrom
fix/desktop-test-build-onboarding
Mar 18, 2026
Merged

Fix desktop test build identity and onboarding recovery#5778
kodjima33 merged 1 commit intomainfrom
fix/desktop-test-build-onboarding

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • align custom macOS desktop test builds so app name, bundle ID, and auth callback scheme stay consistent
  • treat non-production com.omi.* bundles as dev/test builds and preserve custom app naming in the desktop app
  • add a minimal onboarding recovery fallback so resumed onboarding cannot get stuck waiting forever for complete_onboarding
  • remove the stale folder-access onboarding image while keeping real permission guidance, including screen recording/full disk access follow-ups

Verification

  • xcrun swift test -c debug --package-path Desktop
  • built and relaunched /Applications/1233.app
  • verified the running app with agent-swift against bundle id com.omi.1233
  • verified custom callback scheme routing for the 1233 test app during earlier local auth testing

@kodjima33 kodjima33 merged commit 272a2d2 into main Mar 18, 2026
1 check passed
@kodjima33 kodjima33 deleted the fix/desktop-test-build-onboarding branch March 18, 2026 05:19
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 18, 2026

Greptile Summary

This PR tightens the identity contract for macOS desktop test builds — bundle ID, app name, and auth callback URL scheme are now all derived from the same slugify_identifier transform in run.sh and enforced at script startup. The new AppBuild.swift centralises non-production detection under a single isNonProduction check (com.omi.* ≠ production), replacing scattered -dev suffix tests across analytics, logging, and resource monitoring. An onboarding recovery fallback is added to OnboardingChatView that auto-unlocks the "Continue" button 2 seconds after a resumed onboarding session goes idle, preventing users from getting permanently stuck waiting for a complete_onboarding tool call that may never arrive in a restarted session. The remaining file changes are mechanical: alphabetical import ordering and 2-space indentation alignment.

Key changes:

  • run.sh: slugify_identifier derives canonical bundle ID/URL scheme from APP_NAME; strict validation prevents mismatched identifiers from reaching the build; PlistBuddy stamps the custom bundle ID into GoogleService-Info.plist so Firebase accepts the custom app.
  • AppBuild.swift (new): single source of truth for production vs. non-production build detection, display name resolution, and backend URL selection.
  • OnboardingChatView.swift: scheduleRecoveredOnboardingFallback / shouldAutoCompleteRecoveredOnboarding add a 2-second idle fallback that unlocks Continue when file exploration or indexing confirms onboarding was already in progress.
  • AppState.swift: TCC cleanup query broadened from LIKE '%com.omi.desktop%' to LIKE 'com.omi.%' AND ≠ production to cover all custom test bundles.

Issues found:

  • OMI_BUNDLE_ID and OMI_URL_SCHEME env vars are accepted but then immediately rejected unless they match the auto-derived values, making them dead code.
  • The recovery fallback in shouldAutoCompleteRecoveredOnboarding returns true when explorationRunning = true, which can prematurely show "Continue" while a background exploration task is still actively running and appending results to the user's AI profile.

Confidence Score: 3/5

  • PR is mostly safe but has two logic gaps that could affect developer experience and user onboarding quality.
  • The build identity and analytics changes are well-scoped and low-risk. The TCC cleanup broadening is intentional and clearly documented. The two issues — dead OMI_BUNDLE_ID/OMI_URL_SCHEME env vars that exit instead of override, and the recovery fallback unlocking Continue while exploration is still in-flight — are real bugs. The env-var issue will silently break any CI or developer script that set these independently; the exploration-running check could let users skip onboarding before file insights are written to their AI profile. Neither is critical to production users (the non-production flag keeps these paths dev-only), but they should be addressed before the patterns spread.
  • desktop/run.sh (dead env-var validation) and desktop/Desktop/Sources/OnboardingChatView.swift (premature Continue unlock on explorationRunning).

Important Files Changed

Filename Overview
desktop/run.sh Adds slugify_identifier to derive bundle ID and URL scheme from APP_NAME, with strict validation that rejects any override. The OMI_BUNDLE_ID/OMI_URL_SCHEME env vars are set then immediately rejected if they don't match the derived values, making them effectively dead code. Also adds a PlistBuddy step to stamp the custom bundle ID into GoogleService-Info.plist.
desktop/Desktop/Sources/AppBuild.swift New file extracting build-identity helpers (isNonProduction, displayName, backend URL resolution) from scattered per-file checks. isNonProduction broadens the old -dev suffix check to any com.omi.* bundle other than the production ID, enabling all custom test builds to skip analytics automatically.
desktop/Desktop/Sources/OnboardingChatView.swift Adds a recovery fallback (scheduleRecoveredOnboardingFallback / shouldAutoCompleteRecoveredOnboarding) that unlocks the "Continue" button 2 seconds after the chat goes idle when resuming a mid-onboarding session, preventing infinite stuck states. Logic fires on several onChange observers and checks exploration/file-index progress to decide whether to auto-complete.
desktop/Desktop/Sources/AppState.swift Minor changes: broadens TCC cleanup SQL from LIKE '%com.omi.desktop%' to LIKE 'com.omi.%' AND client != 'com.omi.computer-macos' to cover all custom test bundles, plus two cosmetic formatting tweaks.
desktop/Desktop/Sources/OmiApp.swift Import ordering + indentation normalization, plus the windowTitle now reads AppBuild.displayName for all non-production bundles instead of hard-coding the com.omi.desktop-dev check. Preserves custom app naming in the window title for all test builds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["run.sh: OMI_APP_NAME set?"] -->|"'Omi Dev' (default)"| B["EXPECTED_BUNDLE_ID = com.omi.desktop-dev\nEXPECTED_URL_SCHEME = omi-computer-dev"]
    A -->|custom name| C["slugify_identifier(APP_NAME) → slug"]
    C --> D{"slug empty?"}
    D -->|yes| E["exit 1"]
    D -->|no| F["EXPECTED_BUNDLE_ID = com.omi.slug\nEXPECTED_URL_SCHEME = omi-slug"]
    B --> G["Validate OMI_BUNDLE_ID == EXPECTED\nValidate OMI_URL_SCHEME == EXPECTED"]
    F --> G
    G -->|mismatch| H["exit 1 — override rejected"]
    G -->|match| I["Patch Info.plist\nPatch GoogleService-Info.plist BUNDLE_ID"]
    I --> J["Build & sign .app"]
    J --> K["Install to /Applications/APP_NAME.app"]

    subgraph Swift ["Swift runtime (AppBuild.swift)"]
        L["AppBuild.bundleIdentifier"]
        L --> M{"== com.omi.computer-macos?"}
        M -->|yes| N["isNonProduction = false\n→ Analytics ON, Sentry ON"]
        M -->|no| O{"hasPrefix('com.omi.')?"}
        O -->|yes| P["isNonProduction = true\n→ Analytics OFF, dev backend"]
        O -->|no| Q["isNonProduction = false"]
    end

    subgraph Onboarding ["Onboarding Recovery (OnboardingChatView)"]
        R["App relaunch: isMidOnboarding = true"] --> S{"isToolCompleted?"}
        S -->|yes| T["Show Continue immediately"]
        S -->|no| U["Start bridge\nscheduleRecoveredOnboardingFallback"]
        U --> V["2s sleep"]
        V --> W{"explorationRunning\nor explorationCompleted\nor fileCount > 0?"}
        W -->|yes| X["Unlock Continue button"]
        W -->|no| Y["Stay blocked"]
    end
Loading

Comments Outside Diff (3)

  1. desktop/run.sh, line 9723-9736 (link)

    P1 OMI_BUNDLE_ID / OMI_URL_SCHEME overrides are silently rejected

    BUNDLE_ID and URL_SCHEME are populated from the env vars, but the immediately-following guards abort the script if those values differ from the auto-derived expected values. This means the two env vars have become non-functional: setting them to anything other than what slugify_identifier would produce by itself causes an immediate exit 1. A developer who sets OMI_BUNDLE_ID=com.omi.myapp OMI_APP_NAME="My App" will succeed (since slugify gives the same result), but anyone who relied on them as an independent override (e.g. to keep a legacy bundle ID) will now get a hard failure with a non-obvious error message.

    Consider either:

    • Removing the env vars and deriving the values unconditionally (makes the intent clear), or
    • Replacing the strict equality check with an informational warning so operators understand the constraint without a hard exit.
  2. desktop/Desktop/Sources/OnboardingChatView.swift, line 6261-6265 (link)

    P1 Continue unlocked while exploration is still in-flight

    explorationRunning == true means the background file-exploration task is still actively running — the AI hasn't received its results yet. Returning true here causes the 2-second fallback to show the "Continue" button while exploration is in progress, which can allow the user to skip onboarding before the exploration summary is appended to their AI profile.

    The intent of this fallback is to unblock a stuck recovered session, not to show Continue while work is still happening. The condition should be explorationCompleted (finished) rather than explorationRunning (in progress):

  3. desktop/Desktop/Sources/AppBuild.swift, line 2105-2107 (link)

    P2 isNonProduction now silently covers future beta/staging bundles

    The old check (hasSuffix("-dev")) was narrow and deliberate. The new hasPrefix("com.omi.") && != productionBundleIdentifier is broader: any future bundle introduced under com.omi.* (e.g. com.omi.beta, com.omi.staging) will be treated as non-production and will skip analytics, Sentry context uploads, and auto-restart-on-OOM — even if those builds are intended to behave like production. Consider documenting this convention (or adding an explicit allowlist of production-like bundles) so future bundle IDs don't accidentally opt out of monitoring.

Last reviewed commit: "Fix desktop test bui..."

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
…e#5778)

## Summary
- align custom macOS desktop test builds so app name, bundle ID, and
auth callback scheme stay consistent
- treat non-production com.omi.* bundles as dev/test builds and preserve
custom app naming in the desktop app
- add a minimal onboarding recovery fallback so resumed onboarding
cannot get stuck waiting forever for complete_onboarding
- remove the stale folder-access onboarding image while keeping real
permission guidance, including screen recording/full disk access
follow-ups

## Verification
- xcrun swift test -c debug --package-path Desktop
- built and relaunched /Applications/1233.app
- verified the running app with agent-swift against bundle id
com.omi.1233
- verified custom callback scheme routing for the 1233 test app during
earlier local auth testing
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