Migrate the macOS app off ObservableObject to @Observable - #8371
azooz2003-bit wants to merge 14 commits into
Conversation
Wave 1 of the ObservableObject -> @observable migration: 20 leaf state stores (SidebarState, SidebarSelectionState, FileExplorerState, KeyboardShortcutSettingsObserver, ClosedItemHistoryStore, WorkspaceTodoState, hover/visibility singletons, and 13 more) plus all their @StateObject/@ObservedObject/@EnvironmentObject call sites. Remaining Combine subscribers of formerly-@published properties are served by explicit CurrentValueSubject bridges fed from willSet, matching the established tabsPublisher seam semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave 2: TerminalSurface keeps its non-MainActor isolation and nonisolated deinit; only the formerly-@published searchState and keyboardCopyModeActive are tracked, all other storage is @ObservationIgnored so keystroke-hot paths gain no registrar overhead. searchStatePublisher/needlePublisher bridges preserve the exact Published willSet emission timing for the remaining Combine subscribers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave 3: Panel now requires Observation.Observable instead of ObservableObject; all 14 conformers (TerminalPanel, BrowserPanel, CustomSidebarPanel, FilePreviewPanel, MarkdownPanel, ProjectPanel, WorkspaceTodoPanel, RightSidebarToolPanel, CMUXSidebarExtensionBrowserPanel, CloudVMLoadingPanel, AgentSessionPanel) plus BrowserProfileStore, BrowserHistoryStore, and BrowserSearchState migrate with behavioral-parity tracking. Remaining Combine subscribers (Workspace tab-title pipelines, DockSplitStore, tests) consume explicit willSet-fed bridge publishers. BrowserPanel's manual objectWillChange sends are replaced by making the announced state tracked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bservable Wave 4: TerminalNotificationStore, SidebarUnreadModel, CmuxConfigStore, FileExplorerStore, and SessionIndexStore. The SidebarUnreadModel coalescing boundary and SessionIndexStore snapshot boundaries are preserved. FileExplorerStore's manual objectWillChange invalidation is replaced by an explicit coalesced changePublisher that fires on every former invalidation path (tracked-property didSets plus the in-place node mutation sites), keeping the AppKit outline coordinator's 50ms debounced reload contract intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave 5: the Workspace god object drops ObservableObject; its 39 formerly-@published properties become tracked and 146 internal stored members are @ObservationIgnored for behavioral parity. 22 CurrentValueSubject bridges preserve Published willSet semantics for the remaining Combine consumers (sidebar observation, mobile list observer, right-sidebar tools, config, tests). The PaneTreeHosting hooks stop re-emitting objectWillChange since PaneTreeModel's tracked properties are the observation surface; setRemoteTmuxWindowMirror's manual send is replaced by tracking remoteTmuxWindowMirrors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave 6 (final): TabManager drops ObservableObject; the existing tabsPublisher/selectedTabIdPublisher/workspaceGroupsPublisher bridges stay, their hooks stop re-emitting objectWillChange since the tracked WorkspacesModel properties are the observation surface. cmuxApp's remaining @StateObject cluster becomes @State and the AppDelegate environment chain moves to .environment/@Environment. Also splits a two-variable @ObservationIgnored declaration in Workspace.swift that the macro rejects (wave-5 build fix). The app now has zero ObservableObject/@Published/@StateObject/ @ObservedObject/@EnvironmentObject usage in cmux-owned code; the only Combine remnants are documented willSet-fed bridge publishers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion # Conflicts: # Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift # Sources/ContentView.swift # Sources/Panels/BrowserPanel.swift
|
Too many files changed for review. ( Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates application models and SwiftUI observation boundaries from ChangesObservation migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmuxTests/CmuxConfigTests.swift (1)
698-716: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing observation tracking makes the inverted expectation vacuous.
The migration removed the Combine subscription to
$loadedActionshere but did not add the corresponding Observation tracking mechanism (unlike the tests below it). BecausedidAutoReload.fulfill()is never called, this inverted expectation will always pass after 0.25 seconds, even if a hot reload incorrectly occurs.Add
withObservationTrackingto properly assert that the configuration does not auto-reload.💚 Proposed fix
let didAutoReload = expectation(description: "cmux.json should not hot reload") didAutoReload.isInverted = true + withObservationTracking { + _ = store.loadedActions + } onChange: { + didAutoReload.fulfill() + } try """ { "actions": {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmuxTests/CmuxConfigTests.swift` around lines 698 - 716, Update the hot-reload test around the inverted expectation didAutoReload to observe store.loadedActions using withObservationTracking, and fulfill the expectation when the observed value changes. Preserve the existing timeout and assertions so the test fails if configuration observation triggers an automatic reload.Sources/Panels/FilePreviewTextEditor.swift (1)
17-76: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPass
textContentthrough SwiftUI
Sources/Panels/FilePreviewTextEditor.swift:17-76only refreshes the AppKit editor when SwiftUI re-runsupdateNSView, butFilePreviewPanelView.bodynever readspanel.textContent. That leaves clean reload/revert paths able to changetextContentwithout updatingtextView.string, so the editor can stay stale whenisDirtyis already false. Pass the content in as an explicit input or update the text view directly when new text is loaded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Panels/FilePreviewTextEditor.swift` around lines 17 - 76, Ensure SwiftUI observes panel.textContent so FilePreviewTextEditor.updateNSView runs whenever clean reload or revert changes it. Update FilePreviewPanelView.body to pass panel.textContent as an explicit input to FilePreviewTextEditor, preserving the existing textView.string synchronization in updateNSView.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@cmuxTests/CmuxConfigTests.swift`:
- Around line 698-716: Update the hot-reload test around the inverted
expectation didAutoReload to observe store.loadedActions using
withObservationTracking, and fulfill the expectation when the observed value
changes. Preserve the existing timeout and assertions so the test fails if
configuration observation triggers an automatic reload.
In `@Sources/Panels/FilePreviewTextEditor.swift`:
- Around line 17-76: Ensure SwiftUI observes panel.textContent so
FilePreviewTextEditor.updateNSView runs whenever clean reload or revert changes
it. Update FilePreviewPanelView.body to pass panel.textContent as an explicit
input to FilePreviewTextEditor, preserving the existing textView.string
synchronization in updateNSView.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a4e4e8a-056e-40f5-98c5-b25e1aca852a
📒 Files selected for processing (105)
Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/BrowserSurfaceState.swiftPackages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/Scroll/SidebarDragAutoScrollController.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Profiles/Repository/BrowserProfileRepository.swiftPackages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeHosting.swiftPackages/macOS/CmuxPanes/Sources/CmuxPanes/Model/PaneTreeModel.swiftPackages/macOS/CmuxPanes/Sources/CmuxPanes/Model/SplitLayoutModel.swiftPackages/macOS/CmuxPanes/Tests/CmuxPanesTests/PaneTreeModelTests.swiftPackages/macOS/CmuxSidebar/Sources/CmuxSidebar/WorkspaceModel/WorkspaceSidebarMetadataModel.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftPackages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController.swiftPackages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateStateModel.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Core/Model/SurfaceRegistryModel.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesHosting.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Model/WorkspacesModel.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/WorkspacesModelTests.swiftSources/AllShortcutsPopover.swiftSources/App/MenuBarExtraController.swiftSources/AppDelegate.swiftSources/BackgroundWorkspacePrimeCoordinator.swiftSources/BrowserWindowPortal.swiftSources/CMUXSidebarExtensionBrowserPanel.swiftSources/Canvas/WorkspaceCanvasHostView.swiftSources/ClosedItemHistory.swiftSources/CmuxConfig.swiftSources/CoalesceLatestPublisher.swiftSources/ContentView.swiftSources/DiffCommentSubmissionPool.swiftSources/DockSplitStore.swiftSources/Feed/FeedPanelView.swiftSources/Feed/FeedPanelViewModel.swiftSources/FileExplorerState.swiftSources/FileExplorerStore.swiftSources/FileExplorerView.swiftSources/Find/BrowserSearchOverlay.swiftSources/Find/SurfaceSearchOverlay.swiftSources/FocusHistoryMenuInvalidator.swiftSources/FocusSurfaceBroadcaster.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/NotificationsPage.swiftSources/Panels/AgentSessionPanel.swiftSources/Panels/BrowserDesignModePopoverHost.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/CustomSidebarPanel.swiftSources/Panels/CustomSidebarPanelView.swiftSources/Panels/FilePreviewPanel.swiftSources/Panels/FilePreviewTextEditor.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownPanelView.swiftSources/Panels/MarkdownTypographyControl.swiftSources/Panels/PDFPreviewChromeDebugWindowController.swiftSources/Panels/Panel.swiftSources/Panels/PanelContentView.swiftSources/Panels/ProjectBuildSettingsTabView.swiftSources/Panels/ProjectFilesTabView.swiftSources/Panels/ProjectPanel.swiftSources/Panels/ProjectPanelView.swiftSources/Panels/ProjectSchemesTabView.swiftSources/Panels/ProjectTargetsTabView.swiftSources/Panels/TerminalPanel.swiftSources/Panels/TerminalPanelView.swiftSources/Panels/WorkspaceTodoPanel.swiftSources/Panels/WorkspaceTodoPanelView.swiftSources/PricingPlansScreen.swiftSources/RightSidebarPanelView.swiftSources/RightSidebarToolPanel.swiftSources/SessionIndexStore.swiftSources/SessionIndexView.swiftSources/Sidebar/SidebarState.swiftSources/SidebarSelectionState.swiftSources/TabManager.swiftSources/TerminalNotificationStore.swiftSources/TextBoxInput.swiftSources/Update/MinimalModeSidebarControls.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/VerticalTabsSidebar+EmptyAreasAndFooter.swiftSources/WindowDragHandleView.swiftSources/Workspace.swiftSources/WorkspaceContentView.swiftSources/WorkspaceSidebarObservation.swiftSources/WorkspaceTodoState.swiftSources/cmuxApp.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/BrowserConfigTests.swiftcmuxTests/BrowserDesignModeComposerHostingViewTests.swiftcmuxTests/ClosedMainWindowRoutingTests.swiftcmuxTests/CmuxConfigTests.swiftcmuxTests/CmuxDurableDeepLinkRestoreTests.swiftcmuxTests/DockShortcutRoutingTests.swiftcmuxTests/DockTerminalReattachTests.swiftcmuxTests/FileDropOverlayViewTests.swiftcmuxTests/FileExplorerStoreTests.swiftcmuxTests/FocusSurfaceBroadcasterTests.swiftcmuxTests/GhosttyConfigTests.swiftcmuxTests/MarkdownPanelTests.swiftcmuxTests/MobileWorkspaceListFidelityTests.swiftcmuxTests/SessionIndexViewTests.swiftcmuxTests/SidebarLazyLayoutScaleTests.swiftcmuxTests/WindowDockLifecycleTests.swiftcmuxTests/WorkspaceContentViewVisibilityTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftcmuxTests/WorkspaceSidebarObservationTests.swiftcmuxTests/WorkspaceUnitTests.swift
|
Verification status for this first pass:
🤖 Generated with Claude Code |
|
Investigated a dogfood report that sidebar scrolling feels slower on the migrated build. Controlled A/B (Debug vs Debug, this PR's head vs its exact merge-base main commit, 78 seeded workspaces, identical synthetic scroll bursts, one app at a time): median main-thread CPU 11.7% (migrated) vs 9.4% (baseline), ranges overlapping heavily under background load. A One real follow-up found while auditing: the mechanical 🤖 Generated with Claude Code |
# Conflicts: # Sources/TabManager.swift # Sources/Workspace.swift # Sources/cmuxApp.swift
|
Merged latest main (be19996), which includes the AppKit NSTableView sidebar ("Lawrence Sidebar", default ON since #8433). Migration additions in the merge: Verified on the tagged build: AppKit sidebar is the active path ( 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift (1)
84-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExclude continuation waiters from the Observation surface.
sessionRevalidationWaitersis internal coordination state, not UI state. Without@ObservationIgnored, every append/remove can invalidateAuthCoordinatorobservers and trigger unnecessary recomputation. This should matchsignOutCredentialCaptureWaitersat Line 135.- var sessionRevalidationWaiters: [CheckedContinuation<Void, Never>] = [] + `@ObservationIgnored` var sessionRevalidationWaiters: [CheckedContinuation<Void, Never>] = []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift` at line 84, Add `@ObservationIgnored` to the sessionRevalidationWaiters property in AuthCoordinator, matching the existing annotation on signOutCredentialCaptureWaiters, so continuation waiter mutations are excluded from observation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift`:
- Line 84: Add `@ObservationIgnored` to the sessionRevalidationWaiters property in
AuthCoordinator, matching the existing annotation on
signOutCredentialCaptureWaiters, so continuation waiter mutations are excluded
from observation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 39e18fb1-7b7d-4d58-8503-2ff013b1f38c
📒 Files selected for processing (1)
Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift
|
Merge-gate status (2026-07-20, spec merge onto main): web-typecheck now passes (main fixed the next/navigation issue), remote-daemon-tests/react-apps-check/web-db-migrations pass, zero review-bot findings. 🤖 Generated with Claude Code |
An ordinary notification must invalidate observers of the tracked notificationMenuSnapshot.unreadCount projection the titlebar bell badge renders from. Red against the current badge read, which uses the untracked indexes-backed unreadCount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bell badge read TerminalNotificationStore.unreadCount, which is derived from the @ObservationIgnored indexes cache; after dropping ObservableObject the view no longer received whole-store invalidations, so ordinary notification changes never refreshed the badge. Read the tracked, equality-coalesced notificationMenuSnapshot.unreadCount instead (same value, refreshed by refreshUnreadPresentation), matching the popover which already renders from the snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
# Conflicts: # Sources/Workspace.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Update/UpdateTitlebarAccessory.swift`:
- Around line 239-254: Update NotificationsPopoverVisibilityState so the
sourceLessShown state read by isShown(in:) participates in `@Observable` tracking;
remove `@ObservationIgnored` from sourceLessShown or derive isShown(in:) solely
from already-observed state, while preserving source-less show/hide behavior for
non-nil window numbers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6fed9f77-ae35-4113-ac12-39aff6536cc9
📒 Files selected for processing (7)
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftSources/AppDelegate.swiftSources/ContentView.swiftSources/TabManager.swiftSources/TerminalNotificationStore.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/Workspace.swift
| @Observable | ||
| final class NotificationsPopoverVisibilityState { | ||
| static let shared = NotificationsPopoverVisibilityState() | ||
|
|
||
| @Published private(set) var isShown = false | ||
| @Published private(set) var shownWindowNumbers: Set<Int> = [] | ||
| private var shownPopoverIDs: Set<ObjectIdentifier> = [] | ||
| private var shownPopoverWindowNumbers: [ObjectIdentifier: Int] = [:] | ||
| private var sourceLessShown = false | ||
| private(set) var isShown = false | ||
| /// Legacy Combine bridge for the remaining `.$shownWindowNumbers` subscribers. Emits the | ||
| /// new value during willSet and replays the current value on subscribe — the | ||
| /// exact `Published.Publisher` semantics those call sites were written | ||
| /// against. Delete when the subscribers move to @Observable observation. | ||
| @ObservationIgnored let shownWindowNumbersPublisher = CurrentValueSubject<Set<Int>, Never>([]) | ||
| private(set) var shownWindowNumbers: Set<Int> = [] { | ||
| willSet { shownWindowNumbersPublisher.send(newValue) } | ||
| } | ||
| @ObservationIgnored private var shownPopoverIDs: Set<ObjectIdentifier> = [] | ||
| @ObservationIgnored private var shownPopoverWindowNumbers: [ObjectIdentifier: Int] = [:] | ||
| @ObservationIgnored private var sourceLessShown = false |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect isShown(in:) to confirm it reads tracked state (isShown / shownWindowNumbers).
rg -nP -A15 'func isShown\s*\(in' Sources/Update/UpdateTitlebarAccessory.swiftRepository: manaflow-ai/cmux
Length of output: 888
🏁 Script executed:
#!/bin/bash
# Inspect the visibility state implementation and the callers that use isShown(in:).
sed -n '239,340p' Sources/Update/UpdateTitlebarAccessory.swift
printf '\n--- callers ---\n'
rg -n -A4 -B4 'isShown\(in:' Sources/Update/UpdateTitlebarAccessory.swiftRepository: manaflow-ai/cmux
Length of output: 5439
Keep the sourceLessShown branch observable at Sources/Update/UpdateTitlebarAccessory.swift:272-274
isShown(in:) still reads sourceLessShown, but that flag is @ObservationIgnored. For non-nil window numbers, a source-less show/hide can change the result without invalidating the view, so the titlebar controls can stay stale. Track that state, or derive the result from tracked data only.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 240-240: Classes should have an explicit deinit method
(required_deinit)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Update/UpdateTitlebarAccessory.swift` around lines 239 - 254, Update
NotificationsPopoverVisibilityState so the sourceLessShown state read by
isShown(in:) participates in `@Observable` tracking; remove `@ObservationIgnored`
from sourceLessShown or derive isShown(in:) solely from already-observed state,
while preserving source-less show/hide behavior for non-nil window numbers.
…idation Autoreview flagged the removed objectWillChange as a stale fork-menu risk; rejected with evidence: both availability consumers pull at NSMenu construction time, so no invalidation is required. Record the invariant at the site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Autoreview (canonical helper, codex engine, branch mode vs origin/main) ran twice: Round 1 found one real migration regression, now fixed with a red/green pair: the titlebar bell badge read Round 2 on the fixed head found one finding, rejected with evidence: it claimed the removed CodeRabbit's five failed pre-merge checks reference files absent from this PR's three-dot diff (iroh supervisor/release-gate, sidebar probe kit, device-ID helper, localization keys) — they belong to main's own commits transiting during base merges. The one applicable check ("new Combine bridges") is the sanctioned transitional seam per the dual-observation policy: bridges keep exact Published timing while consumer migration to withObservationTracking is deferred to follow-ups. Merge-conflict gate: clean both rounds. Head: e850334. 🤖 Generated with Claude Code |
The @observable macro emits its Observable conformance extension at file scope; a private class nested inside a test class is inaccessible there, which broke the cmuxTests build on the first restored CI run (DetachedWorkspaceTestPanel and five sibling mocks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Full merge gate is green on the speculative merge (run https://github.com/manaflow-ai/cmux/actions/runs/29866305381): all four app-host unit-test shards, E2E Merge remains gated only on the owner's dogfood verdict. 🤖 Generated with Claude Code |
lawrencecchen
added
the
stale-revisit
3 participants
Replaces every remaining
ObservableObjectconformance in cmux-owned code with the@Observablemacro: 42 types across the app target and the CmuxAppKitSupportUI/CmuxTerminal packages, including thePanelprotocol (now requiresObservation.Observable) and theWorkspace/TabManagergod objects. SwiftUI now invalidates views per accessed property instead of on everyobjectWillChange, which removes the whole-store redraw traffic that@Publishedfan-out caused.Mechanics, wave by wave (one commit each):
TerminalSurface+SearchState(keeps its non-MainActor isolation and nonisolated deinit).Panelprotocol + all 14 conformers and the browser stores.TerminalNotificationStore/SidebarUnreadModel,CmuxConfigStore,FileExplorerStore,SessionIndexStore.Workspace(39 tracked properties, 22 bridge publishers, 146@ObservationIgnoredinternals).TabManager+ app environment plumbing, final sweep.Behavioral-parity rules used throughout: only formerly-
@Publishedproperties are tracked and every other stored member is@ObservationIgnored, so redraw semantics and hot-path costs match the old world; remaining Combine subscribers consume explicitCurrentValueSubjectbridges fed fromwillSet(the establishedtabsPublisherseam pattern), so emission timing and replay-on-subscribe are identical; manualobjectWillChange.send()invalidation was replaced by tracking the announced state, andFileExplorerStore's AppKit outline coordinator keeps its 50ms-debounced reload via an explicitchangePublisherthat fires on every former invalidation path. View wrappers moved@StateObject→@State,@ObservedObject→let/@Bindable,@EnvironmentObject→@Environment(T.self);.onReceive($prop)sites became.onChange(initial: true)to preserve the projected publisher's replay delivery. TheTabItemViewEquatable typing-latency gate and the sidebar snapshot boundaries are untouched.Zero
ObservableObject/@Published/@StateObject/@ObservedObject/@EnvironmentObject/objectWillChangeuses remain in cmux-owned macOS and iOS sources (the iOS app was already fully on@Observable).Supersedes #7606, which predates the sub-model refactor waves this migration builds on.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Migrates the macOS app fully to
@Observable, includingWorkspace,TabManager, all panels, and core stores. Also fixes the titlebar unread badge by reading the trackednotificationMenuSnapshot.unreadCount, and updates tests to keep@Observablemacro scoping happy.Refactors
ObservableObjectwith@Observableacross app and packages;Panelnow requiresObservation.Observable. Migrated all panel types andTerminalSurfacewhile keeping its isolation and nonisolated deinit.@Bindableand@Environment(Type.self); updated app root and tests. InjectedBrowserDesignModeCardDragBridgefrom the host.notificationMenuSnapshotPublisher,pendingBackgroundWorkspaceLoadIdsPublisher, BrowserPanelpageTitlePublisher/isLoadingPublisher/faviconPNGDataPublisher/isMutedPublisher/webViewLifecycleStatePublisher, minimal‑modehoveredWindowNumberPublisher, notificationsshownWindowNumbersPublisher). Kept the file explorer’s debounced outline reload viaFileExplorerStore.changePublisher.@Observable(notification, config, file explorer, session index, keyboard shortcuts, closed‑item history, sidebar drag auto‑scroll, workspace todo). Documented that the agent‑index notification availability task is pull‑only at NSMenu construction, so no global invalidation is required.@Observabletest mocks tofileprivateto satisfy the macro’s file‑scope conformance emission and fixed tests to observe tracked properties instead ofobjectWillChange.Migration
.environmentObject(X)with.environment(X)and read via@Environment(X.self).@ObservedObjectwithlet/@Bindableand@StateObjectwith@State.$propsubscribers for the provided bridge...Publishers where Combine remains.Written for commit 5c57c3e. Summary will update on new commits.
Summary by CodeRabbit