Open
Conversation
- Migrate all onChange(of:perform:) calls to new macOS 14+ API (zero or two parameter closures) - Replace deprecated activateIgnoringOtherApps with NSApp.activate() - Replace deprecated fullPath(forApplication:) with availability-silenced helper - Remove deprecated WKProcessPool usage (no effect since macOS 12) - Add @preconcurrency imports for Dispatch, ObjectiveC, UserNotifications - Add @preconcurrency to UNUserNotificationCenterDelegate and BonsplitDelegate conformances - Wrap main-actor-isolated calls in NotificationCenter/DispatchQueue callbacks with MainActor.assumeIsolated - Mark cross-isolation captures as nonisolated(unsafe) where thread-safe - Add @discardableResult to closePanel to suppress ~17 unused result warnings - Fix unused variables, unnecessary casts, non-optional nil coalescing - Handle exhaustive switch for new TabContextAction cases - Extract deprecated CGWindowListCreateImage into availability-annotated helper - Suppress Sparkle SUAppcastItem deprecation via availability wrapper Remaining 5 warnings are intentionally unfixable without major refactoring (no replacement API, ScreenCaptureKit migration, standard observer pattern).
|
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:
📝 WalkthroughWalkthroughCentralized app activation calls to Changes
Sequence Diagram(s)mermaid mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryThis PR reduces ~250 compiler warnings to 5 by migrating
Confidence Score: 4/5Not safe to merge as-is: the MainActor.assumeIsolated inside Thread.detachNewThread will crash the app on every socket connection. 14 of 15 changed files are straightforward, correct warning suppressions. The single exception in TerminalController.swift is a P0 logic error that will cause a hard crash (preconditionFailure) on every socket command — the primary CLI/automation path. Fix is a one-liner. Sources/TerminalController.swift — the Thread.detachNewThread + MainActor.assumeIsolated combination at line 1505.
|
| Filename | Overview |
|---|---|
| Sources/TerminalController.swift | Adds @preconcurrency import and nonisolated(unsafe) properties — sound — but wraps handleClient call in MainActor.assumeIsolated inside Thread.detachNewThread, which is a guaranteed runtime crash on every socket connection. |
| Sources/Workspace.swift | Adds @preconcurrency ObjectiveC import, MainActor.assumeIsolated for NotificationCenter callbacks on .main queue (correct), nonisolated(unsafe) for observer/static vars, @discardableResult for closePanel, and no-op break cases for the two new TabContextAction enum values. |
| Sources/AppDelegate.swift | Adds @preconcurrency Dispatch/ObjectiveC.runtime imports, @preconcurrency UNUserNotificationCenterDelegate, nonisolated(unsafe) static shared, replaces activateIgnoringOtherApps with NSApp.activate(), and wraps main-queue callbacks in MainActor.assumeIsolated — all correct. |
| Sources/GhosttyTerminalView.swift | Multiple MainActor.assumeIsolated wraps in callbacks already dispatched to .main queue; nonisolated(unsafe) captures for cross-isolation — all correct. |
| Sources/cmuxApp.swift | Migrates all onChange(of:perform:) calls to the zero/two-parameter macOS 14+ API; adds @discardableResult to resolvedMode — straightforward and low-risk. |
| Sources/ContentView.swift | Migrates onChange(of:perform:) calls to new API and wraps main-queue NotificationCenter callbacks with MainActor.assumeIsolated — correct. |
| Sources/TerminalNotificationStore.swift | Adds @preconcurrency UserNotifications import, wraps .main-queue observer callback with MainActor.assumeIsolated, and adds nonisolated(unsafe) let for the UNNotificationContent capture — all sound. |
| Sources/Update/UpdateTestSupport.swift | Wraps deprecated SUAppcastItem(dictionary:) call in an @available(*, deprecated) helper to silence Sparkle's own deprecation warning without changing behavior — clean approach. |
| Sources/Panels/BrowserPanel.swift | Uses MainActor.assumeIsolated in callbacks already dispatched on .main; adds availability-annotated helper to silence CGWindowListCreateImage deprecation — correct. |
| Sources/Panels/BrowserPanelView.swift | Migrates onChange(of:perform:) to new API and other concurrency warning fixes — low-risk mechanical changes. |
| Sources/KeyboardShortcutSettings.swift | Replaces activateIgnoringOtherApps with NSApp.activate() — correct and non-breaking on macOS 14+. |
Sequence Diagram
sequenceDiagram
participant AcceptLoop as Accept Loop (background thread)
participant DetachedThread as Thread.detachNewThread
participant MainActor as @MainActor (main thread)
AcceptLoop->>DetachedThread: accept() returns clientSocket
DetachedThread->>DetachedThread: MainActor.assumeIsolated { handleClient(...) }
Note over DetachedThread: ❌ preconditionFailure crash!<br/>Not running on main actor
Note over AcceptLoop,MainActor: Correct fix
AcceptLoop->>MainActor: DispatchQueue.main.async { handleClient(...) }
MainActor->>MainActor: handleClient(clientSocket, peerPid:) ✅
Reviews (1): Last reviewed commit: "Fix ~245 compiler warnings across 15 sou..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
1505
to
1509
| Thread.detachNewThread { [weak self] in | ||
| self?.handleClient(clientSocket, peerPid: peerPid) | ||
| MainActor.assumeIsolated { | ||
| self?.handleClient(clientSocket, peerPid: peerPid) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
MainActor.assumeIsolated inside a detached thread will crash at runtime
Thread.detachNewThread spawns a new OS thread that is not the main actor. MainActor.assumeIsolated calls preconditionFailure (a hard crash) when the current executor is not the main actor — so this will crash on every incoming socket connection. The concurrency warning should be resolved by dispatching to main instead:
Suggested change
| Thread.detachNewThread { [weak self] in | |
| self?.handleClient(clientSocket, peerPid: peerPid) | |
| MainActor.assumeIsolated { | |
| self?.handleClient(clientSocket, peerPid: peerPid) | |
| } | |
| } | |
| Thread.detachNewThread { [weak self] in | |
| DispatchQueue.main.async { | |
| self?.handleClient(clientSocket, peerPid: peerPid) | |
| } | |
| } |
Or, since handleClient runs on @MainActor anyway, the thread dispatch can be removed entirely and replaced with a plain DispatchQueue.main.async.
Comment on lines
+12164
to
+12165
| case .moveToLeftPane, .moveToRightPane: | ||
| break |
Contributor
There was a problem hiding this comment.
moveToLeftPane / moveToRightPane silenced but unimplemented
These new TabContextAction cases are handled with break, which satisfies the compiler but means any UI element that triggers them is silently ignored. If the enum cases are user-visible in the tab context menu, users will tap them and get no response. Consider adding a TODO comment or an assertion so the gap is easy to spot, or filing a follow-up issue and noting it here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6123dab3d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1505
to
+1507
| Thread.detachNewThread { [weak self] in | ||
| self?.handleClient(clientSocket, peerPid: peerPid) | ||
| MainActor.assumeIsolated { | ||
| self?.handleClient(clientSocket, peerPid: peerPid) |
There was a problem hiding this comment.
Thread.detachNewThread runs this closure on a background thread, but MainActor.assumeIsolated requires the current executor to already be the main actor. On real socket traffic this can trigger actor-precondition failures (or undefined cross-actor access) when handling a client, so socket commands can crash or behave nondeterministically. Use an actual hop (Task { @MainActor ... }/DispatchQueue.main.async) instead of assumeIsolated here.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed in b441a5d. Reverted to direct call.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Fixed in b441a5d. Reverted to direct call.
— Claude Code
Comment on lines
2511
to
2515
| static func configureWebViewConfiguration( | ||
| _ configuration: WKWebViewConfiguration, | ||
| websiteDataStore: WKWebsiteDataStore, | ||
| processPool: WKProcessPool = BrowserPanel.sharedProcessPool | ||
| websiteDataStore: WKWebsiteDataStore | ||
| ) { | ||
| configuration.processPool = processPool | ||
| configuration.mediaTypesRequiringUserActionForPlayback = [] |
There was a problem hiding this comment.
This configuration path no longer sets WKWebViewConfiguration.processPool, so each browser/popup web view falls back to its own default pool. That breaks shared in-memory web session state between panels (notably session cookies/login state), causing repeated auth/CAPTCHA prompts when opening new tabs/popups in the same run. The previous shared-pool behavior was required for cross-panel cookie sharing and should be preserved.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Not applicable. WKProcessPool has had no effect since macOS 12 and never handled cookies or sessions. Cookie persistence is handled by WKWebsiteDataStore, which is still set.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Not applicable. WKProcessPool has had no effect since macOS 12 and never handled cookies or sessions. Cookie persistence is handled by WKWebsiteDataStore, which is still set.
— Claude Code
Comment on lines
+850
to
+852
| self.center.add(request) { error in | ||
| if let error { | ||
| NSLog("Failed to schedule test notification: \(error)") | ||
| self.logAuthorization("settings test schedule failed error=\(error.localizedDescription)") | ||
| } else { | ||
| self.logAuthorization("settings test schedule succeeded") | ||
| NotificationSoundSettings.runCustomCommand( | ||
| title: content.title, | ||
| subtitle: content.subtitle, | ||
| body: content.body | ||
| ) | ||
| MainActor.assumeIsolated { | ||
| if let error { |
There was a problem hiding this comment.
The completion handler of UNUserNotificationCenter.add is not guaranteed to run on the main actor, so wrapping it with MainActor.assumeIsolated can trip actor checks when the callback arrives on a background queue. This turns a settings test-notification path into a potential runtime crash; use a real main-actor hop in the callback body instead.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed. Replaced MainActor.assumeIsolated with DispatchQueue.main.async since UNUserNotificationCenter's completion handler runs on an arbitrary queue.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Fixed. Replaced MainActor.assumeIsolated with DispatchQueue.main.async since UNUserNotificationCenter completion handler runs on an arbitrary queue.
— Claude Code
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/TerminalController.swift">
<violation number="1" location="Sources/TerminalController.swift:1506">
P1: `MainActor.assumeIsolated` is used from a detached background thread, which can violate main-actor isolation for `handleClient` and cause race/crash behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/AppDelegate.swift (1)
12615-12620:⚠️ Potential issue | 🟠 MajorAvoid re-activating the app from duplicate-launch handling.
This observer runs for non-focus-intent launches too, so
NSApp.activate()can yank focus away from the user just because a second instance was rejected. I’d drop the activation here or gate it behind an explicit user-intent path.Suggested change
app.terminate() if !app.isTerminated { _ = app.forceTerminate() } - NSApp.activate() }As per coding guidelines: "Only explicit focus-intent commands may mutate in-app focus/selection:
window.focus,workspace.select/next/previous/last,surface.focus,pane.focus/last, browser focus commands, and v1 focus equivalents. All other commands must preserve current user focus context while applying data/model changes"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 12615 - 12620, The duplicate-launch handler should not steal focus by calling NSApp.activate(); remove the NSApp.activate() call (or wrap it behind an explicit user-focus-intent check) in the duplicate-launch handling block that calls app.terminate() and app.forceTerminate() in AppDelegate.swift so that rejecting a second instance does not change app focus; locate the code invoking app.terminate(), checking app.isTerminated and calling app.forceTerminate(), and delete or gate the NSApp.activate() invocation accordingly (only allow activation when an explicit focus-intent path is detected).Sources/Panels/BrowserPanel.swift (1)
9293-9297:⚠️ Potential issue | 🟡 MinorReplace deprecated
NSApp.activate(ignoringOtherApps:)withNSApp.activate().The
ignoringOtherAppsparameter is deprecated on macOS 14+. Change line 9296 to useNSApp.activate()to match the modern pattern already being used elsewhere in the codebase (KeyboardShortcutSettings.swift, TerminalController.swift, AppDelegate.swift). The window ordering viamakeKeyAndOrderFront(nil)already handles window state independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 9293 - 9297, In runModal(), replace the deprecated call NSApp.activate(ignoringOtherApps: true) with the modern NSApp.activate() call; locate the runModal() function (around the panel.center() / panel.makeKeyAndOrderFront(nil) sequence) and change that single line so activation uses NSApp.activate() to match other usages like KeyboardShortcutSettings and AppDelegate.
🧹 Nitpick comments (1)
Sources/TerminalNotificationStore.swift (1)
1159-1168: Prefer immutable value capture overnonisolated(unsafe)here.Capturing
title/subtitle/bodyas plainStringvalues avoids unsafe cross-isolation escape ofUNMutableNotificationContent.♻️ Suggested refactor
- nonisolated(unsafe) let safeContent = content + let safeTitle = content.title + let safeSubtitle = content.subtitle + let safeBody = content.body self.center.add(request) { error in if let error { NSLog("Failed to schedule notification: \(error)") } else { NotificationSoundSettings.runCustomCommand( - title: safeContent.title, - subtitle: safeContent.subtitle, - body: safeContent.body + title: safeTitle, + subtitle: safeSubtitle, + body: safeBody ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalNotificationStore.swift` around lines 1159 - 1168, Replace the unsafe cross-isolation capture of UNMutableNotificationContent by extracting immutable String values before the closure: remove the nonisolated(unsafe) let safeContent = content and instead read content.title, content.subtitle and content.body into local let constants (e.g., title, subtitle, body) in the same isolation context, then inside self.center.add(request) { error in ... } call NotificationSoundSettings.runCustomCommand(title: title, subtitle: subtitle, body: body); this keeps only plain String captures and avoids escaping the notification content across isolation boundaries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 476-483: Remove the `@available`(..., deprecated: ...) annotation
from the helper _legacyFullPath(forApplication:) so the wrapper itself is not
marked deprecated; leave the deprecation warning on the call to
NSWorkspace.shared.fullPath(forApplication:) inside the function body so the
warning is localized to the wrapper, and apply the same removal to the other
similar wrapper in the file that follows the same pattern.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 7797-7800: Remove the unsafe MainActor.assumeIsolated default
parameter usage in static func realize and resolve BrowserProfileStore.shared
from inside the function body instead: change the parameter signature to not
call MainActor.assumeIsolated (e.g. make profileStore optional or require it),
then at the start of realize (which runs on the main actor) assign a local let
profileStore = providedProfileStore ?? BrowserProfileStore.shared so the shared
instance is accessed under main-actor isolation; references: realize,
BrowserProfileStore.shared, MainActor.assumeIsolated.
In `@Sources/TerminalController.swift`:
- Around line 1505-1509: Summary: You're calling MainActor.assumeIsolated from a
background thread (spawned by Thread.detachNewThread), which incorrectly asserts
main-actor context instead of dispatching to it. Fix: when spawning the
background thread (Thread.detachNewThread) don't call MainActor.assumeIsolated;
instead explicitly schedule handleClient on the main actor — for example, create
a Task that uses await MainActor.run { self?.handleClient(clientSocket, peerPid:
peerPid) } or dispatch via DispatchQueue.main.async { self?.handleClient(...) }
so handleClient executes on the main actor. Locate Thread.detachNewThread and
replace the MainActor.assumeIsolated usage with an explicit main-actor dispatch
targeting handleClient.
In `@Sources/TerminalNotificationStore.swift`:
- Around line 849-863: The completion block passed to self.center.add currently
calls MainActor.assumeIsolated (unsafe) which may run off the main actor;
replace that with wrapping the completion body in a Task { `@MainActor` in ... }
so the UI/main-actor work (calls to self.logAuthorization and
NotificationSoundSettings.runCustomCommand) executes on the main actor safely;
keep the nonisolated(unsafe) let safeContent capture as before and reference
safeContent, self.logAuthorization, and
NotificationSoundSettings.runCustomCommand inside the Task to preserve behavior.
In `@Sources/Update/UpdateTestSupport.swift`:
- Around line 101-103: The wrapper method makeAppcastItem calls the deprecated
helper _makeAppcastItem from a non-deprecated function which produces a
deprecation warning; to suppress that warning, annotate makeAppcastItem with
`@available`(*, deprecated) so the wrapper is also treated deprecated and its
internal call to _makeAppcastItem won’t emit warnings, keeping the
implementation identical (i.e., still return _makeAppcastItem(displayVersion:
displayVersion)).
---
Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 12615-12620: The duplicate-launch handler should not steal focus
by calling NSApp.activate(); remove the NSApp.activate() call (or wrap it behind
an explicit user-focus-intent check) in the duplicate-launch handling block that
calls app.terminate() and app.forceTerminate() in AppDelegate.swift so that
rejecting a second instance does not change app focus; locate the code invoking
app.terminate(), checking app.isTerminated and calling app.forceTerminate(), and
delete or gate the NSApp.activate() invocation accordingly (only allow
activation when an explicit focus-intent path is detected).
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 9293-9297: In runModal(), replace the deprecated call
NSApp.activate(ignoringOtherApps: true) with the modern NSApp.activate() call;
locate the runModal() function (around the panel.center() /
panel.makeKeyAndOrderFront(nil) sequence) and change that single line so
activation uses NSApp.activate() to match other usages like
KeyboardShortcutSettings and AppDelegate.
---
Nitpick comments:
In `@Sources/TerminalNotificationStore.swift`:
- Around line 1159-1168: Replace the unsafe cross-isolation capture of
UNMutableNotificationContent by extracting immutable String values before the
closure: remove the nonisolated(unsafe) let safeContent = content and instead
read content.title, content.subtitle and content.body into local let constants
(e.g., title, subtitle, body) in the same isolation context, then inside
self.center.add(request) { error in ... } call
NotificationSoundSettings.runCustomCommand(title: title, subtitle: subtitle,
body: body); this keeps only plain String captures and avoids escaping the
notification content across isolation boundaries.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1964a781-e364-4cfc-acfe-d554caba9988
📒 Files selected for processing (15)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettings.swiftSources/NotificationsPage.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/BrowserPopupWindowController.swiftSources/Panels/MarkdownPanelView.swiftSources/TerminalController.swiftSources/TerminalNotificationStore.swiftSources/Update/UpdateLogStore.swiftSources/Update/UpdateTestSupport.swiftSources/Workspace.swiftSources/cmuxApp.swift
Comment on lines
+476
to
+483
| /// Wraps the deprecated `NSWorkspace.fullPath(forApplication:)` to suppress | ||
| /// the deprecation warning at call sites. No non-deprecated name-based | ||
| /// replacement exists; `urlForApplication(withBundleIdentifier:)` requires a | ||
| /// bundle identifier, not an application display name. | ||
| @available(macOS, deprecated: 11.0) | ||
| private func _legacyFullPath(forApplication name: String) -> String? { | ||
| NSWorkspace.shared.fullPath(forApplication: name) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Swift, if a wrapper function is annotated @available(macOS, deprecated: 11.0), do calls to that wrapper emit deprecation warnings at each call site?
💡 Result:
Yes. In Swift, a declaration marked with @available(..., deprecated: ...) will emit a deprecation warning at every reference/use site of that declaration—so if you put the deprecation on your wrapper function, then each call to the wrapper will warn. (forums.swift.org)
Citations:
Don't mark the wrapper itself deprecated.
When a function is marked @available(..., deprecated: ...), deprecation warnings are emitted at every call site, not just inside the wrapper. To localize the unavoidable warning to the wrapper body, keep the helper unmarked and let the warning from NSWorkspace.shared.fullPath(forApplication:) stay contained within it.
Suggested change
-@available(macOS, deprecated: 11.0)
private func _legacyFullPath(forApplication name: String) -> String? {
NSWorkspace.shared.fullPath(forApplication: name)
}Also applies to: 512
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 476 - 483, Remove the `@available`(...,
deprecated: ...) annotation from the helper _legacyFullPath(forApplication:) so
the wrapper itself is not marked deprecated; leave the deprecation warning on
the call to NSWorkspace.shared.fullPath(forApplication:) inside the function
body so the warning is localized to the wrapper, and apply the same removal to
the other similar wrapper in the file that follows the same pattern.
Contributor
Author
There was a problem hiding this comment.
Acknowledged. The wrapper consolidates the deprecated call to one location. The remaining warning at the call site is intentional since there is no non-deprecated name-based replacement API.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Acknowledged. The wrapper consolidates the deprecated call. The remaining warning at the call site is intentional since no non-deprecated name-based replacement API exists.
— Claude Code
There was a problem hiding this comment.
@lawrencecchen Got it. Keeping the wrapper deprecated to centralize usage and preserve a single intentional warning makes sense here given there’s no name-based replacement API. No further change from me.
Tiny nit for future clarity: the doc comment says “suppress the deprecation warning at call sites,” but since the wrapper itself is deprecated, calls to it will warn; consider tweaking that sentence so it matches the intended behavior. Otherwise, all good.
✏️ Learnings added
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2735
File: Sources/AppDelegate.swift:476-483
Timestamp: 2026-04-09T00:29:03.675Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — The helper `_legacyFullPath(forApplication:)` intentionally remains annotated `available(macOS, deprecated: 11.0)` and calls `NSWorkspace.shared.fullPath(forApplication:)`. Team preference: keep a single, localized deprecation warning at its (single) call site since no non‑deprecated name-based replacement exists. Do not “fix” this by removing the deprecation or silencing the warning in future PRs.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2034
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-25T07:14:56.211Z
Learning: Repo: manaflow-ai/cmux — Sources/AppDelegate.swift — Pattern for “Open Folder”: In AppDelegate.showOpenFolderPanel(), seed NSOpenPanel.directoryURL using preferredMainWindowContextForWorkspaceCreation(debugSource: …) rather than NSApp.keyWindow, and on selection delegate to openWorkspaceForExternalDirectory(workingDirectory:…, debugSource: …). Rationale: handles auxiliary-key-window cases, ensures shouldBringToFront = true, and unifies menu/shortcut behavior with a consistent fallback to createMainWindow when workspace creation returns nil.
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 1980
File: Sources/AppDelegate.swift:2454-2458
Timestamp: 2026-04-06T09:33:40.789Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — In activateUITestAppIfNeeded() for macOS 14+, prefer NSRunningApplication.current.activate(options: [.activateAllWindows]) without .activateIgnoringOtherApps; the deprecated option is only used on older macOS. A future follow-up may migrate to the modern activation API.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2124
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-25T08:05:26.034Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — Pattern for “New Window” geometry seed:
In AppDelegate.createMainWindow(initialWorkingDirectory:sessionWindowSnapshot:), compute the existingFrame using preferredMainWindowContextForWorkspaceCreation(debugSource: "createMainWindow.initialGeometry") and then resolvedWindow(for:) rather than relying on NSApp.keyWindow or the first registered mainWindowContext. Rationale: ensures the new window inherits size from the intended main-terminal window even when an auxiliary window is key, and keeps behavior consistent with showOpenFolderPanel().
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 1980
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-04-06T09:33:43.688Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — UI test launch stabilization: in stabilizeUITestLaunchWindowAndForeground(attempt:), call moveUITestWindowToTargetDisplayIfNeeded() only on the first pass (attempt == 0) so the helper’s own 20-step retry handles display moves; subsequent attempts only activate the app and write diagnostics. This avoids nested overlapping retries and stage-log churn.
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 2043
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-04-06T09:32:26.967Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — UI test fallback window pattern: during XCTest launch stabilization, the app may force-create a fallback main window when NSApp.windows is empty. The window is tracked via `private weak var fallbackUITestWindow: NSWindow?` and, upon the first real WindowGroup registration in `registerMainWindow(...)`, if a second main window exists and the registering window differs, the fallback is closed and the reference cleared to avoid two main windows during tests.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-08T03:36:30.148Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.FileBrowserDrawerState threading pattern (PR `#1909`, commit e0e57809): FileBrowserDrawerState must be threaded through AppDelegate.configure() as a weak stored property (matching the sidebarState pattern), passed through both configure() call sites, with registerMainWindow parameter made non-optional. The fallback `?? FileBrowserDrawerState()` must NOT be used as it creates detached instances that are not properly owned by the window context.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:4910-4918
Timestamp: 2026-04-06T02:02:45.662Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — Keyboard repair pattern: In AppDelegate.repairFocusedTerminalKeyboardRoutingIfNeeded(window:event:), determine whether to repair focus by asking the focused terminal’s hosted view if the current first responder already matches that panel’s preferred keyboard target. Implemented via hostedView.responderMatchesPreferredKeyboardFocus(responder) inside responderNeedsFocusedTerminalKeyRepair(_:in:hostedView:). This covers same-window drift to a different Ghostty surface without comparing workspace/panel IDs. Verified by test cmuxTests/AppDelegateShortcutRoutingTests.swift::testWindowSendEventRepairsVisibleSameWindowResponderDriftForFocusedTerminalTyping.
Learnt from: Horacehxw
Repo: manaflow-ai/cmux PR: 2043
File: Sources/AppDelegate.swift:2414-2447
Timestamp: 2026-04-06T09:51:56.134Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — UI test launch call-site: In applicationDidFinishLaunching(_:), when running under XCTest, call stabilizeUITestLaunchWindowAndForeground(attempt: 0) instead of a one‑shot asyncAfter block. The stabilizer handles a single fallback window creation, attempt==0‑gated display move, app activation, diagnostics, and retries.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:5636-5642
Timestamp: 2026-04-06T07:18:38.685Z
Learning: In the AppDelegate.swift keyDown focus-repair path (and related surface keyboard-focus matching like GhosttySurfaceScrollView.swift), do not dereference NSTextView.delegate when it is unsafe-unretained. Instead, resolve the field-editor ownership via cmuxFieldEditorOwnerView(_). For keyboard-focus matching, prefer superview/nextResponder traversal or hostedView.responderMatchesPreferredKeyboardFocus(...), and ensure any matches are performed using the owned responder chain rather than reading an unsafe delegate pointer.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T02:19:07.311Z
Learning: Applies to **/*.swift : For telemetry hot paths: Parse and validate arguments off-main, dedupe/coalesce off-main first, and schedule minimal UI/model mutation with `DispatchQueue.main.async` only when needed
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-02T21:40:59.098Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift session snapshot logic, detected foreground command lines are only recorded if they pass SessionRestoreCommandSettings.isCommandAllowed(...). Commit f887189 moved gating to the SessionForegroundProcessCache so unallowed (potentially sensitive) commands are never written to the session JSON.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.710Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/AppDelegate.swift:2196-2200
Timestamp: 2026-04-03T03:36:45.112Z
Learning: Repo: manaflow-ai/cmux — In Sources/AppDelegate.swift, when KeyboardShortcutSettings.didChangeNotification fires, AppDelegate must clear configured-chord caches (pendingConfiguredShortcutChord and activeConfiguredShortcutChordPrefixForCurrentEvent) via clearConfiguredShortcutChordState() before refreshing tooltips/UI. Also clear chord state on applicationWillResignActive to avoid cross-activity leakage. Verified by cmuxTests/AppDelegateShortcutRoutingTests.swift::testShortcutChangeClearsPendingConfiguredChord.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2662
File: Sources/TabManager.swift:1994-1995
Timestamp: 2026-04-07T05:27:46.372Z
Learning: Repo: manaflow-ai/cmux — Sources/TabManager.swift — In applyWorkspacePullRequestRefreshResults(...), resolution .unsupportedRepository must (1) clear any existing workspace.panelPullRequests[panelId] entry and (2) reset lastTerminalState timestamp. scheduleNextWorkspacePullRequestPoll(...) sets nextPollAt to a jittered background interval for .unsupportedRepository, avoiding hot-looping `git remote -v`. Intent: when a panel moves to a non-GitHub or otherwise unsupported repo, clear the sidebar PR badge and back off polling.
Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2564
File: Sources/Workspace.swift:1072-1075
Timestamp: 2026-04-04T02:21:20.453Z
Learning: Repo: manaflow-ai/cmux — Foreground-auth deferral: In CLI/cmux.swift, PermitLocalCommand and LocalCommand are injected only into the foreground startup SSH command (e.g., via deferredRemoteReconnectLocalCommand(...)) and are not added to the ssh_options payload sent in workspace.remote.configure. Consequently, WorkspaceRemoteSSHBatchCommandBuilder.batchArguments(...) and WorkspaceRemoteSessionController.backgroundSSHOptions(_:) do not need to filter LocalCommand/PermitLocalCommand for batch operations.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:12.296Z
Learning: In PRs affecting this repository, limit the scope of localization (i18n) changes to Japanese translations for the file Sources/AppDelegate.swift. Do not include UX enhancements (e.g., preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels) in this PR. Open a separate follow-up issue to address any UX-related changes to avoid scope creep and keep localization review focused.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:05:52.379Z
Learning: In Sources/AppDelegate.swift (within the manaflow-ai/cmux repo), ensure that NSWindow.cmux_performKeyEquivalent forwards to firstResponderWebView.performKeyEquivalent and returns its Bool unconditionally. This prevents re-entry into SwiftUI’s performKeyEquivalent path that can swallow keys when WKWebView has focus, and aligns with the Command-key routing chain implemented in Sources/Panels/CmuxWebView.swift: (1) route to NSApp.mainMenu.performKeyEquivalent when allowed, (2) fall back to AppDelegate.shared?.handleBrowserSurfaceKeyEquivalent(event) for non-menu shortcuts, (3) fall back to super.performKeyEquivalent. Non-Command keys should still call super directly.
Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:05:52.379Z
Learning: In Sources/AppDelegate.swift for the manaflow-ai/cmux repository, Cmd+` (command-backtick) should not be routed directly to the main menu. Do not implement a window/main-menu bypass for this key. Tests will assert this behavior, so ensure routing logic respects this exclusion and that there is no path that bypasses the main menu for Cmd+`. If similar key-command bypass considerations exist in other files, apply the same explicit exclusion only where applicable.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Comment on lines
+101
to
+103
| private static func makeAppcastItem(displayVersion: String) -> SUAppcastItem? { | ||
| _makeAppcastItem(displayVersion: displayVersion) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Swift (current Xcode toolchain), does calling a function annotated with @available(*, deprecated) from a non-deprecated function emit a deprecation warning at the call site?
💡 Result:
Yes. In the current Swift/Xcode toolchain, a declaration marked @available(*, deprecated, ...) triggers a deprecation warning at each call site in non-deprecated code (i.e., wherever you call it from a non-deprecated function/method). Deprecation warnings are “generated at the callsite.” [1]
If the caller is also annotated as deprecated (e.g., you mark the wrapping function @available(*, deprecated)), then calls within that deprecated scope won’t emit additional deprecation warnings, which is a common way to build a deprecated shim API without “polluting” warnings internally. [2]
Sources: [1] [2]
Mark the wrapper as deprecated to suppress the warning at call site
Line 102 calls the deprecated helper from a non-deprecated function, which will emit a deprecation warning in the current Swift toolchain. To suppress warnings without polluting the wrapper function's declaration, mark makeAppcastItem as @available(*, deprecated) as well, so internal calls to _makeAppcastItem won't trigger warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Update/UpdateTestSupport.swift` around lines 101 - 103, The wrapper
method makeAppcastItem calls the deprecated helper _makeAppcastItem from a
non-deprecated function which produces a deprecation warning; to suppress that
warning, annotate makeAppcastItem with `@available`(*, deprecated) so the wrapper
is also treated deprecated and its internal call to _makeAppcastItem won’t emit
warnings, keeping the implementation identical (i.e., still return
_makeAppcastItem(displayVersion: displayVersion)).
Contributor
Author
There was a problem hiding this comment.
Acknowledged. Same pattern as the fullPath wrapper. The Sparkle API has no replacement yet.
— Claude Code
Contributor
Author
There was a problem hiding this comment.
Acknowledged. Same pattern as the fullPath wrapper. Sparkle API has no replacement yet.
— Claude Code
There was a problem hiding this comment.
@lawrencecchen, understood — the single-layer wrapper (makeAppcastItem → _makeAppcastItem) is the chosen pattern to isolate the deprecation to one location while Sparkle provides no replacement. I'll note this for future reviews.
✏️ Learnings added
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2735
File: Sources/Update/UpdateTestSupport.swift:101-103
Timestamp: 2026-04-09T00:28:47.592Z
Learning: Repo: manaflow-ai/cmux — In `Sources/Update/UpdateTestSupport.swift`, the pattern of a non-deprecated `makeAppcastItem(displayVersion:)` wrapper that calls the `available(*, deprecated)` helper `_makeAppcastItem(displayVersion:)` is intentional. It isolates the Sparkle `SUAppcastItem(dictionary:)` deprecation warning to a single site because Sparkle provides no replacement API yet. Do not flag this call site as a warning; the design mirrors the `_legacyFullPath(forApplication:)` wrapper in `Sources/AppDelegate.swift`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-08T03:36:30.148Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.FileBrowserDrawerState threading pattern (PR `#1909`, commit e0e57809): FileBrowserDrawerState must be threaded through AppDelegate.configure() as a weak stored property (matching the sidebarState pattern), passed through both configure() call sites, with registerMainWindow parameter made non-optional. The fallback `?? FileBrowserDrawerState()` must NOT be used as it creates detached instances that are not properly owned by the window context.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.710Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/AppDelegate.swift:2196-2200
Timestamp: 2026-04-03T03:36:45.112Z
Learning: Repo: manaflow-ai/cmux — In Sources/AppDelegate.swift, when KeyboardShortcutSettings.didChangeNotification fires, AppDelegate must clear configured-chord caches (pendingConfiguredShortcutChord and activeConfiguredShortcutChordPrefixForCurrentEvent) via clearConfiguredShortcutChordState() before refreshing tooltips/UI. Also clear chord state on applicationWillResignActive to avoid cross-activity leakage. Verified by cmuxTests/AppDelegateShortcutRoutingTests.swift::testShortcutChangeClearsPendingConfiguredChord.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:5636-5642
Timestamp: 2026-04-06T07:18:41.310Z
Learning: Repo: manaflow-ai/cmux — In AppDelegate’s .keyDown focus-repair path, never dereference NSTextView.delegate (unsafe-unretained). Resolve field-editor ownership via cmuxFieldEditorOwnerView(_), and prefer superview/nextResponder traversal or hostedView.responderMatchesPreferredKeyboardFocus(...) for matching, as applied in AppDelegate.swift and GhosttySurfaceScrollView.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-02T21:40:59.098Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift session snapshot logic, detected foreground command lines are only recorded if they pass SessionRestoreCommandSettings.isCommandAllowed(...). Commit f887189 moved gating to the SessionForegroundProcessCache so unallowed (potentially sensitive) commands are never written to the session JSON.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2662
File: Sources/TabManager.swift:1994-1995
Timestamp: 2026-04-07T05:27:46.372Z
Learning: Repo: manaflow-ai/cmux — Sources/TabManager.swift — In applyWorkspacePullRequestRefreshResults(...), resolution .unsupportedRepository must (1) clear any existing workspace.panelPullRequests[panelId] entry and (2) reset lastTerminalState timestamp. scheduleNextWorkspacePullRequestPoll(...) sets nextPollAt to a jittered background interval for .unsupportedRepository, avoiding hot-looping `git remote -v`. Intent: when a panel moves to a non-GitHub or otherwise unsupported repo, clear the sidebar PR badge and back off polling.
Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/AppDelegate.swift:1955-1970
Timestamp: 2026-03-21T07:13:41.796Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.registerMainWindow ownership pattern: the primary window registers from ContentView.onAppear and passes the SwiftUI-owned FileBrowserDrawerState; secondary windows created via AppDelegate.createMainWindow construct and pass their own FileBrowserDrawerState. This mirrors SidebarState and avoids re-registration mismatches.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
There was a problem hiding this comment.
@lawrencecchen, understood — the intentional wrapper indirection (non-deprecated public surface calling a @available(*, deprecated) private helper) is the same silencing pattern used for _legacyFullPath(forApplication:), and is appropriate here since Sparkle has not yet provided a replacement for SUAppcastItem(dictionary:). The one trade-off is that the call site in makeAppcastItem will still emit a deprecation warning from the compiler; if that becomes noisy, the fix would simply be to add @available(*, deprecated) to makeAppcastItem as well. Happy to leave it as-is given the current state of the Sparkle API.
🧠 Learnings used
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-08T03:36:30.148Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.FileBrowserDrawerState threading pattern (PR `#1909`, commit e0e57809): FileBrowserDrawerState must be threaded through AppDelegate.configure() as a weak stored property (matching the sidebarState pattern), passed through both configure() call sites, with registerMainWindow parameter made non-optional. The fallback `?? FileBrowserDrawerState()` must NOT be used as it creates detached instances that are not properly owned by the window context.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.710Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/AppDelegate.swift:2196-2200
Timestamp: 2026-04-03T03:36:45.112Z
Learning: Repo: manaflow-ai/cmux — In Sources/AppDelegate.swift, when KeyboardShortcutSettings.didChangeNotification fires, AppDelegate must clear configured-chord caches (pendingConfiguredShortcutChord and activeConfiguredShortcutChordPrefixForCurrentEvent) via clearConfiguredShortcutChordState() before refreshing tooltips/UI. Also clear chord state on applicationWillResignActive to avoid cross-activity leakage. Verified by cmuxTests/AppDelegateShortcutRoutingTests.swift::testShortcutChangeClearsPendingConfiguredChord.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:5636-5642
Timestamp: 2026-04-06T07:18:41.310Z
Learning: Repo: manaflow-ai/cmux — In AppDelegate’s .keyDown focus-repair path, never dereference NSTextView.delegate (unsafe-unretained). Resolve field-editor ownership via cmuxFieldEditorOwnerView(_), and prefer superview/nextResponder traversal or hostedView.responderMatchesPreferredKeyboardFocus(...) for matching, as applied in AppDelegate.swift and GhosttySurfaceScrollView.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-02T21:40:59.098Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift session snapshot logic, detected foreground command lines are only recorded if they pass SessionRestoreCommandSettings.isCommandAllowed(...). Commit f887189 moved gating to the SessionForegroundProcessCache so unallowed (potentially sensitive) commands are never written to the session JSON.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2662
File: Sources/TabManager.swift:1994-1995
Timestamp: 2026-04-07T05:27:46.372Z
Learning: Repo: manaflow-ai/cmux — Sources/TabManager.swift — In applyWorkspacePullRequestRefreshResults(...), resolution .unsupportedRepository must (1) clear any existing workspace.panelPullRequests[panelId] entry and (2) reset lastTerminalState timestamp. scheduleNextWorkspacePullRequestPoll(...) sets nextPollAt to a jittered background interval for .unsupportedRepository, avoiding hot-looping `git remote -v`. Intent: when a panel moves to a non-GitHub or otherwise unsupported repo, clear the sidebar PR badge and back off polling.
Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/AppDelegate.swift:1955-1970
Timestamp: 2026-03-21T07:13:41.796Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.registerMainWindow ownership pattern: the primary window registers from ContentView.onAppear and passes the SwiftUI-owned FileBrowserDrawerState; secondary windows created via AppDelegate.createMainWindow construct and pass their own FileBrowserDrawerState. This mirrors SidebarState and avoids re-registration mismatches.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
The socket accept loop runs on a detached background thread, not the main thread. MainActor.assumeIsolated traps when called off-main. Reverted to the original direct call pattern which was correct.
- BrowserPanel: remove redundant MainActor.assumeIsolated from @mainactor function's default parameter (already on main actor) - TerminalNotificationStore: replace MainActor.assumeIsolated with DispatchQueue.main.async in UNUserNotificationCenter completion handler (runs on arbitrary queue, not guaranteed main thread)
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Sources/Panels/BrowserPanel.swift (2)
2511-2514: Update the process-pool comment to match this change.Now that
configureWebViewConfigurationno longer accepts or assigns aWKProcessPool, the nearby class doc at Line 1704 is stale. Leaving it as-is will push future maintenance back toward an assumption this PR just removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 2511 - 2514, The class doc near the top of BrowserPanel that still mentions assigning or accepting a WKProcessPool is now stale because configureWebViewConfiguration(_:websiteDataStore:) no longer takes or sets a WKProcessPool; update that documentation to remove references to WKProcessPool and to describe the current behavior of configureWebViewConfiguration(_:websiteDataStore:) (e.g., it configures WKWebViewConfiguration and uses the provided WKWebsiteDataStore) and any implications for process pooling or isolation so the doc matches the new API.
1843-1848: Drop the redundantMainActor.assumeIsolatedhere.
BrowserPanelis already@MainActor, so this static method inherits main-actor isolation by default per Swift Evolution SE-0316. TheassumeIsolatedwrapper is unnecessary and only makes the code harder to reason about.♻️ Proposed cleanup
private static func isDarkAppearance( appAppearance: NSAppearance? = nil ) -> Bool { - let appearance: NSAppearance? = appAppearance ?? MainActor.assumeIsolated { NSApp?.effectiveAppearance } + let appearance: NSAppearance? = appAppearance ?? NSApp?.effectiveAppearance guard let appearance else { return false } return appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 1843 - 1848, The isDarkAppearance(appAppearance:) static method is redundantly calling MainActor.assumeIsolated; remove the assumeIsolated wrapper and directly access NSApp?.effectiveAppearance because BrowserPanel is already `@MainActor` and the static method inherits main-actor isolation. Update the implementation of isDarkAppearance to use let appearance: NSAppearance? = appAppearance ?? NSApp?.effectiveAppearance, keep the existing guard and bestMatch logic, and ensure no other uses of MainActor.assumeIsolated remain in that function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 2511-2514: The class doc near the top of BrowserPanel that still
mentions assigning or accepting a WKProcessPool is now stale because
configureWebViewConfiguration(_:websiteDataStore:) no longer takes or sets a
WKProcessPool; update that documentation to remove references to WKProcessPool
and to describe the current behavior of
configureWebViewConfiguration(_:websiteDataStore:) (e.g., it configures
WKWebViewConfiguration and uses the provided WKWebsiteDataStore) and any
implications for process pooling or isolation so the doc matches the new API.
- Around line 1843-1848: The isDarkAppearance(appAppearance:) static method is
redundantly calling MainActor.assumeIsolated; remove the assumeIsolated wrapper
and directly access NSApp?.effectiveAppearance because BrowserPanel is already
`@MainActor` and the static method inherits main-actor isolation. Update the
implementation of isDarkAppearance to use let appearance: NSAppearance? =
appAppearance ?? NSApp?.effectiveAppearance, keep the existing guard and
bestMatch logic, and ensure no other uses of MainActor.assumeIsolated remain in
that function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bdbf4e9-5484-4c76-af43-efe5c42469b4
📒 Files selected for processing (2)
Sources/Panels/BrowserPanel.swiftSources/TerminalNotificationStore.swift
- Update class doc to reference WKWebsiteDataStore instead of removed WKProcessPool - Remove MainActor.assumeIsolated wrapper from isDarkAppearance, access NSApp?.effectiveAppearance directly (eliminates NSAppearance Sendable warning)
NSApp.activate() is cooperative and requires the foreground app to yield, which is unreliable for socket commands, keyboard shortcuts, and bringToFront. Introduced NSApplication.forceActivate() wrapper that uses activateIgnoringOtherApps(true) with an intentional deprecation annotation documenting why cooperative activation is insufficient for these paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 13918-13927: Remove the `@available`(macOS, deprecated: 14.0, ...)
annotation from the extension NSApplication wrapper func forceActivate(), so the
wrapper itself is not marked deprecated; keep the doc comment explaining
rationale and leave the call to activate(ignoringOtherApps: true) intact (the
deprecation warning will still appear inside the function body due to the
underlying API).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6acbca48-6cb5-46df-b6e9-1ddba94c2e64
📒 Files selected for processing (3)
Sources/AppDelegate.swiftSources/KeyboardShortcutSettings.swiftSources/TerminalController.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/TerminalController.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dacfc125ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> Bool, | ||
| to: ghostty_runtime_read_clipboard_cb.self | ||
| ) | ||
| runtimeConfig.read_clipboard_cb = cmuxRuntimeReadClipboardCallback |
There was a problem hiding this comment.
Replacing the explicit cast with runtimeConfig.read_clipboard_cb = cmuxRuntimeReadClipboardCallback drops the compatibility shim for GhosttyKit builds that import read_clipboard_cb with a Void return type instead of Bool (the mismatch noted in the adjacent comment). In that environment this assignment is not type-compatible and the project no longer compiles, so this change narrows supported build configurations.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
The unsafeBitCast removal is safe. The compiler warned it was unnecessary because the types already match in the current GhosttyKit build. The comment about cross-importer variants is preserved, and if a future build changes the callback signature, it will produce a compile error (not a silent ABI mismatch).
— Claude Code
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:13923">
P3: `forceActivate()` is marked deprecated, so every new call site now produces a deprecation warning in this module. If the goal is to reduce compiler warnings, drop the deprecation attribute and keep the intent in the doc comment instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Despite Apple's deprecation message ("creating multiple instances no
longer has any effect"), removing the shared WKProcessPool entirely
broke in-memory session/cookie state sharing between browser panels
and popup flows. Restored the shared pool with a deprecation annotation
documenting why it must be kept.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b718c2d635
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> Bool, | ||
| to: ghostty_runtime_read_clipboard_cb.self | ||
| ) | ||
| runtimeConfig.read_clipboard_cb = cmuxRuntimeReadClipboardCallback |
There was a problem hiding this comment.
Assigning cmuxRuntimeReadClipboardCallback directly narrows support to GhosttyKit importer variants where ghostty_runtime_read_clipboard_cb has a Bool return in Swift. In environments where that callback is imported as Void (the compatibility case documented in the adjacent comment), this line becomes type-incompatible and the project no longer compiles; the previous explicit C-ABI cast/shim avoided that mismatch.
Useful? React with 👍 / 👎.
…edProcessPool - Use @objc message send for activateIgnoringOtherApps to avoid deprecation warning propagation to all 7 call sites - Remove @available annotation from sharedProcessPool (the comment explains why it's kept, the annotation was counterproductive)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4a21a3fab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // unreliable for focus-intent paths. No replacement exists. | ||
| // Using @objc message send to avoid deprecation warning propagation. | ||
| let sel = NSSelectorFromString("activateIgnoringOtherApps:") | ||
| perform(sel, with: true as NSNumber) |
There was a problem hiding this comment.
forceActivate() currently uses perform(_:with:) to call activateIgnoringOtherApps:, but perform only supports object-typed arguments and this selector expects a BOOL. That is an ABI mismatch (undefined behavior): the method reads a scalar while we pass an object pointer, so focus-intent paths can intermittently fail to activate the app depending on the pointer value layout. Please call the API through a typed Bool invocation (in a locally deprecated-silenced wrapper) instead of perform.
Useful? React with 👍 / 👎.
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
1 participant
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.
Summary
Reduces compiler warnings from ~250 to 5 (all intentionally unfixable without major refactoring).
onChange(of:perform:)calls to the macOS 14+ API across cmuxApp, ContentView, BrowserPanelView, NotificationsPage, MarkdownPanelViewactivateIgnoringOtherApps,fullPath(forApplication:),WKProcessPool,CGWindowListCreateImage@preconcurrencyimports,MainActor.assumeIsolatedfor known-main-thread callbacks,nonisolated(unsafe)for safe cross-isolation captures@discardableResulttoclosePanel(_:force:)to suppress 17 unused result warningsTabContextActioncases (moveToLeftPane,moveToRightPane)5 remaining warnings are intentional: no replacement API exists (
fullPath), ScreenCaptureKit migration is a separate task (CGWindowListCreateImage), standard observer self-removal pattern,NSAppearanceSendable conformance, andevaluateJavaScriptasync suggestion.Testing
./scripts/reload.sh --tag fix-warningscmux DEV fix-warnings.appSummary by cubic
Reduces compiler warnings from ~250 to 5 by moving to macOS 14+ APIs, wrapping deprecated calls, and tightening Swift concurrency. Restores reliable activation for focus-intent paths and shared browser session state, while silencing deprecation noise at call sites.
Refactors
onChangeusages to the macOS 14 two-arg/zero-arg forms.NSApp.forceActivate()(Obj‑C selector callsactivateIgnoringOtherApps:) and adopted it across sockets/shortcuts/bring-to-front for reliable focus without deprecation noise._legacyFullPath(forApplication:); wrapped window capture in an availability-silenced helper; restored sharedWKProcessPoolandWKWebsiteDataStorefor session/cookie persistence; added a SparkleSUAppcastItemwrapper to suppress deprecation.@preconcurrencyimports/conformances; limitedMainActor.assumeIsolatedto safe paths; dispatchedUNUserNotificationCentercompletions to the main queue; marked safe capturesnonisolated(unsafe)and closures@Sendable.unsafeBitCastshim for cross-importer callback compatibility.Workspace.closePanel(_:force:)as@discardableResult; removed unsafe casts; fixed unused vars; made switches exhaustive for newTabContextActioncases. Remaining warnings are intentional (no safe replacements or deferred migrations).Bug Fixes
MainActor.assumeIsolatedfrom the background socket accept loop to prevent a crash and restore thread-safety.Written for commit 19c4251. Summary will update on new commits.
Summary by CodeRabbit
Chores
Refactor
Style