Skip to content

Fix API key race: wait inside ensureBridgeStarted, not callers#6014

Merged
kodjima33 merged 1 commit into
mainfrom
worktree-fix-onboarding-restart-recovery
Mar 24, 2026
Merged

Fix API key race: wait inside ensureBridgeStarted, not callers#6014
kodjima33 merged 1 commit into
mainfrom
worktree-fix-onboarding-restart-recovery

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

Previous fix (#5987) added waitForKeys() in OnboardingChatView before sendMessage(), but the bridge was already started by ViewModelContainer.warmupBridge()ensureBridgeStarted() — 26ms before keys loaded. The bridge launched in Mode B (no API key) every time.

Root cause: ensureBridgeStarted() calls acpBridge.start() which spawns a subprocess that reads ANTHROPIC_API_KEY from environment. If the key isn't set yet, it starts in OAuth mode — which fails for new users who don't have a Claude account.

Fix: Move waitForKeys() into ensureBridgeStarted() itself, right before acpBridge.start(). One place, covers all callers. Also store fetchTask from OmiApp launch so waitForKeys() can always await it.

Test plan

  • Clean install → sign in → verify bridge starts in Mode A (check log for "Mode A (Omi API key)")
  • Verify onboarding chat works without Claude OAuth session

🤖 Generated with Claude Code

The previous fix added waitForKeys() in OnboardingChatView before
sendMessage(), but the bridge was already started earlier by
ViewModelContainer.warmupBridge() → ensureBridgeStarted(). By the time
our wait ran, the bridge was alive in Mode B (no key).

Fix: move the wait into ensureBridgeStarted() itself — one place,
covers all callers (warmup, sendMessage, anything). Also store the
fetchTask from OmiApp launch so waitForKeys() can always await it.

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

greptile-apps Bot commented Mar 24, 2026

Greptile Summary

This PR consolidates the waitForKeys() call into ensureBridgeStarted() itself so the ACP bridge always waits for ANTHROPIC_API_KEY to be set before launching the subprocess, fixing the race condition where the bridge started in Mode B (OAuth) instead of Mode A (Omi API key). It also stores the fetchTask handle on APIKeyService so that waitForKeys() can always await the in-flight fetch regardless of which code path triggered the bridge start.

Changes:

  • APIKeyService.swiftprivate var fetchTask changed to private(set) var fetchTask; startFetchingKeys() helper removed.
  • OmiApp.swift — assigns APIKeyService.shared.fetchTask directly instead of spawning a fire-and-forget Task {}.
  • OnboardingChatView.swift — removes both waitForKeys() call-sites (bridge warmup path and fresh-start sendMessage path).
  • ChatProvider.swift — adds waitForKeys() inside ensureBridgeStarted(), guarded by acpBridge.passApiKey, as the single canonical wait point.

Issue found:

  • fetchTask is declared private(set), but OmiApp.swift writes to it from a different file — Swift's private(set) restricts the setter to the enclosing type and same-file extensions only, so this will not compile. The setter access level must be changed to internal (i.e., just var fetchTask).

Confidence Score: 3/5

  • Not safe to merge as-is — the private(set) access modifier on fetchTask will prevent the module from compiling.
  • The logic of the fix is correct and the architectural approach (centralising the wait inside ensureBridgeStarted) is sound. However, the access-control change on APIKeyService.fetchTask (private varprivate(set) var) introduces a hard compile error: Swift's private(set) only allows the setter within the enclosing type/same-file extensions, yet OmiApp.swift writes to it from AppDelegate. Changing private(set) to plain var (internal setter) is a one-line fix, after which the PR should be safe to merge.
  • desktop/Desktop/Sources/APIKeyService.swift — setter access level must be corrected from private(set) to var before this compiles.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/APIKeyService.swift Changed private var fetchTask to private(set) var fetchTask, but private(set) still prevents OmiApp.swift from writing to the property — this will cause a compile error.
desktop/Desktop/Sources/OmiApp.swift Stores the fetch task in APIKeyService.shared.fetchTask so waitForKeys() can always await it. Assignment will fail to compile unless fetchTask's setter access level is corrected in APIKeyService.swift.
desktop/Desktop/Sources/OnboardingChatView.swift Correctly removes the two waitForKeys() calls that were previously needed before bridge warmup and sendMessage; comment updated to point to internal handling in ensureBridgeStarted.
desktop/Desktop/Sources/Providers/ChatProvider.swift Adds waitForKeys() inside ensureBridgeStarted() guarded by acpBridge.passApiKey, placing the wait at the correct single chokepoint before the subprocess is launched.

Sequence Diagram

sequenceDiagram
    participant OmiApp
    participant APIKeyService
    participant ChatProvider
    participant ACPBridge

    OmiApp->>APIKeyService: fetchTask = Task { fetchKeys() }
    Note over APIKeyService: fetchTask stored for awaiting

    OmiApp->>ChatProvider: warmupBridge() [via ViewModelContainer]
    ChatProvider->>ChatProvider: ensureBridgeStarted()
    alt acpBridge.passApiKey == true
        ChatProvider->>APIKeyService: waitForKeys()
        APIKeyService-->>ChatProvider: (keys loaded)
    end
    ChatProvider->>ACPBridge: start()
    Note over ACPBridge: Reads ANTHROPIC_API_KEY<br/>→ launches in Mode A

    OmiApp->>ChatProvider: sendMessage(...)
    ChatProvider->>ChatProvider: ensureBridgeStarted()
    Note over ChatProvider: guard !acpBridgeStarted → returns true immediately
Loading

Comments Outside Diff (1)

  1. desktop/Desktop/Sources/APIKeyService.swift, line 23 (link)

    P0 private(set) blocks external write — compile error

    OmiApp.swift sets APIKeyService.shared.fetchTask = Task { ... } from AppDelegate, which is a different file. In Swift, private(set) restricts the setter to the enclosing declaration and same-file extensions only. External files like OmiApp.swift cannot call the setter, so this will produce a compiler error:

    error: cannot assign to property: 'fetchTask' setter is inaccessible

    The setter access level should be removed (defaulting to internal) so that OmiApp.swift can assign it:

Reviews (1): Last reviewed commit: "fix(desktop): wait for API keys inside e..." | Re-trigger Greptile

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

## Summary
Previous fix (BasedHardware#5987) added `waitForKeys()` in OnboardingChatView before
`sendMessage()`, but the bridge was already started by
`ViewModelContainer.warmupBridge()` → `ensureBridgeStarted()` — 26ms
before keys loaded. The bridge launched in Mode B (no API key) every
time.

**Root cause**: `ensureBridgeStarted()` calls `acpBridge.start()` which
spawns a subprocess that reads `ANTHROPIC_API_KEY` from environment. If
the key isn't set yet, it starts in OAuth mode — which fails for new
users who don't have a Claude account.

**Fix**: Move `waitForKeys()` into `ensureBridgeStarted()` itself, right
before `acpBridge.start()`. One place, covers all callers. Also store
`fetchTask` from OmiApp launch so `waitForKeys()` can always await it.

## Test plan
- [ ] Clean install → sign in → verify bridge starts in Mode A (check
log for "Mode A (Omi API key)")
- [ ] Verify onboarding chat works without Claude OAuth session

🤖 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