fix(desktop): macOS bug audit cycle 3 follow-up — Bee audio, conversation cache, installer atomicity, floating bar, WAL sync#10047
Merged
Conversation
processAudioPacket popped only the first complete ADTS frame per notification; a notification that completed two frames (common after a resync) emitted frame 2 only when the NEXT notification arrived, shifting all later audio ~64ms/frame late (cumulative) and discarding surplus complete frames on stop. It now drains every complete frame in a while-let loop; a genuinely partial trailing frame still stays buffered for the next notification. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
The cache-first load() published bare cached rows without overlaying in-flight optimistic mutations, so a load() that raced a pending star/title/folder edit (e.g. the user toggled a filter mid-mutation) visually reverted the edit until the remote call landed. The cache emit now overlays pendingMutations per row via ConversationReconciliationPolicy.apply, mirroring the server merge path. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
…t brick it The DMG-install path did removeItem(destination) then copyItem(source → destination); a copy failure after the remove (e.g. ENOSPC off a near-full DMG) destroyed the user's working /Applications/Omi.app, leaving only the DMG copy. replaceInstalledApp now stages the fresh bundle beside the destination and swaps it in with replaceItemAt only after the copy succeeds, so a failed copy leaves the existing install untouched. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
savePreChatCenterIfNeeded snapped the draggable pill to the pending restore origin paired with the bare collapsed pill size, but that origin was computed for the glow-inflated window. Each rapid open→close→re-open recorded a center offset by one glow outset (~22pt left / 18pt down), compounding every cycle. It now stores the full restore frame (origin + inflated size) and snaps to it, so the recorded center round-trips exactly. Restore-origin math routes through pure FloatingControlBarGeometry helpers covered by a round-trip test. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
…tion cleanup Two independent WAL cloud-sync defects: - fetchSyncJobStatus swallowed an auth-header build failure with try? and sent an UNAUTHENTICATED status GET, and mapped HTTP 401 to .transient. A token-mint failure (account transition race / locked Keychain) or a stale 401 therefore parked the WAL in a forever-retry. It now returns a distinct .unauthorized outcome without issuing an unauthenticated request (pure forStatusCode mapping), and the reconciler reverts the WAL to .miss for re-upload through the authenticated path — the poll never invalidates the session (INV-AUTH-1). - cleanupOldWals was dead code (never called), so already-synced WAL audio + metadata grew without bound. It now runs at the end of each syncToCloud pass, selecting reclaimable entries via a pure WALCloudSyncLogic.cleanupCandidates (synced-only, past the 7-day retention window) and using the injected clock. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
…ty test TasksStore.localOnlyRestoreRecord is a @mainactor static; the test class is not MainActor-isolated, so the call is implicitly async and must be awaited. This test (added in #10035) merged before its Swift CI finished, leaving main's test target red; the await fixes the compile and unblocks the build. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
testCleanupRemovesOldSyncedWalButKeepsYoungAndUnsynced gave the old .synced and old .miss entries the same (device, timerStart), so they shared the WAL id dev1_<ts>. cleanupOldWals removes by id, so deleting the old synced entry also removed the old miss entry with the identical id, failing the "unsynced WAL is kept" assertion. Give the .miss entry a distinct device so ids differ (production reserves unique timerStarts, so real WALs never collide). Test-only fixture fix. Failure-Class: none Co-Authored-By: Claude <noreply@anthropic.com>
eulicesl
marked this pull request as ready for review
July 19, 2026 21:53
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
…s-testing-i8z7cm # Conflicts: # desktop/macos/Desktop/Tests/ActionItemLocalIdentityMutationTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
macOS bug audit — cycle 3 follow-up (deferred low-severity set)
Follow-up to the merged cycle-3 audit (#10035). Cycle 3 fixed the high-severity crash/data-loss/leak/security findings and consciously deferred a set of lower-severity items. This PR completes the deferred findings that have a clean, hermetically-testable seam — each re-verified against current
mainbefore fixing.Fixes
Bee audio frame drift (C6) —
processAudioPacketpopped only the first ADTS frame per BLE notification; a notification completing two frames emitted the second one notification late (~64ms/frame, cumulative) and discarded surplus complete frames on stop. Now drains every complete frame per notification. Regression test feeds a two-frame packet and asserts both drain in order.Conversation optimistic-edit revert (C24) — the cache-first
load()published bare cached rows without overlaying pending optimistic mutations, so a filter change mid-mutation visually reverted an in-flight star/title/folder edit. Cache emit now overlayspendingMutations(mirrors the server merge path). Hermetic test suspends a star mutation and asserts the cache reload keeps it.Non-atomic app install (C14) — the DMG install did
removeItemthencopyItem; a copy failure after the remove destroyed the user's working/Applications/Omi.app.replaceInstalledAppnow stages thenreplaceItemAt-swaps, leaving the existing install untouched on failure. Real-FileManager temp-dir tests assert survival on copy failure + successful swap.Floating-bar pill drift (C10) —
savePreChatCenterIfNeededsnapped the draggable pill's restore origin (computed for the glow-inflated window) using the bare pill size, drifting the recorded center ~22pt left / 18pt down per rapid re-open cycle. Now stores + snaps the full restore frame; restore-origin math routes through pureFloatingControlBarGeometryhelpers with a round-trip identity test.WAL sync stuck on auth failure (C1) —
fetchSyncJobStatusswallowed an auth-header failure withtry?(sending an unauthenticated GET) and mapped 401 →.transient, parking the WAL in a forever-retry. Now returns a distinct.unauthorized(pureforStatusCodemapping) without an unauthenticated request; the reconciler reverts to.missfor re-upload via the authenticated path (poll stays session-preserving, INV-AUTH-1).Unbounded WAL retention (C19) —
cleanupOldWalswas dead code, so synced audio + metadata grew without bound. Now invoked at the end of eachsyncToCloud, selecting reclaimable entries via a pureWALCloudSyncLogic.cleanupCandidates(synced-only, past the 7-day window) using the injected clock.Verification
check_desktop_test_quality.py, line-count ratchet (FloatingControlBarWindow baseline raised with justification), e2e flow-coverage (AppInstaller added todesktop-update-relaunchcovers).Product invariants affected
C1 names INV-AUTH-1 because the WAL status poll is a background, session-preserving request: on an auth failure it throws
.unauthorizedand reverts the WAL for re-upload through the authenticated path — it does not invalidate the Firebase session (that ownership stays withAuthSessionCoordinator). No session-death path changes.INV-CHAT-1 is named because the C10 pill-drift fix touches
FloatingControlBarWindow.swift/FloatingControlBarGeometry.swift(path globs). The change is presentation geometry only — it stores and snaps the full restore frame instead of pairing an inflated origin with the bare pill size. It does not touch any chat write-path surface: noChatProvider, kernel journal/projection, floating viewport cursor, turn identity, or pill projection code changes. The floating bar's chat continuity contract is unaffected — only where the collapsed pill window comes to rest on screen.Failure class (fixes)
Failure-Class: none
Independent instance fixes across unrelated subsystems (Bluetooth audio, conversations cache, installer, floating bar, WAL sync); none recurs against a registered failure class.
Deferred (not in this PR)
AudioSourceManager: unreachable dead code; the correct resolution is delete + relocate the still-liveAudioSourceenum across many files, which needs a local Swift compile.🤖 Generated with Claude Code
Generated by Claude Code