Repository navigation
Add configurable file extension openers - #5040
lawrencecchen wants to merge 11 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds per-file-extension command-click routing behavior configuration. It introduces a new ChangesFile Extension-Based Command-Click Open Behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (14 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: 7d9b70653e
ℹ️ 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".
| @@ -0,0 +1,33 @@ | |||
| import Foundation | |||
|
|
|||
| public enum FileExtensionOpenBehavior: String, CaseIterable, Identifiable, Sendable, SettingCodable { | |||
There was a problem hiding this comment.
The repository's AGENTS.md requires every public symbol added under Packages/ to have Swift-DocC /// documentation, including types, enum cases, properties, and methods. This new public enum and its public members are currently undocumented, so the package API violates the documented review contract and will leave generated package docs incomplete; please add DocC comments for the enum, cases, id, defaultOpeners, and normalizedExtension.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR routes Cmd-clicked
Confidence Score: 5/5Safe to merge; routing logic is well-tested, the concurrency migration is correct, and fallback chains are consistent across all call sites. The routing refactor is straightforward: a well-covered Action enum replaces ad-hoc boolean checks, the concurrency migration (DispatchQueue → Task @mainactor) is correct, and the cmux.json parser now uses continue so a single bad entry no longer silently drops the rest of the app block. The only open items are documentation clarity for the automatic semantic on built-in extensions and missing catalog translations for 18 non-en/ja locales, both of which are non-blocking. Resources/Localizable.xcstrings — all new user-facing strings need translations for the 18 locales beyond en and ja that the catalog already supports. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Cmd-click file path] --> B{shouldHandleCommandClick}
B -- false --> Z[System/Ghostty handles link]
B -- true --> C[deferredOpenCommandClickFile]
C --> D[action for path]
D --> E{fileExtensionOpeners behavior?}
E -- cmuxBrowser --> F[openOrFocusBrowserSplit]
E -- cmuxPreview --> G[openOrFocusFilePreviewSplit]
E -- markdownViewer --> H[openOrFocusMarkdownSplit]
E -- preferredEditor --> I[PreferredEditorSettings.open]
E -- systemDefault --> J[NSWorkspace.shared.open]
E -- "automatic + built-in ext (html/htm)" --> K[.unhandled → fallback]
E -- "automatic + other ext or no entry" --> L[Legacy routing]
L --> M{CmdClickMarkdownRouteSettings}
M -- yes --> H
M -- no --> N{CmdClickSupportedFileRouteSettings}
N -- yes --> G
N -- no --> K
F -- nil/fail --> O[fallback.open]
G -- nil/fail --> O
H -- nil/fail --> O
K --> O
Reviews (8): Last reviewed commit: "Document file opening settings section" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (rawExtension, rawBehavior) in values { | ||
| guard let normalizedExtension = FileExtensionOpenBehavior.normalizedExtension(rawExtension), | ||
| let behavior = FileExtensionOpenBehavior(rawValue: rawBehavior) else { | ||
| logInvalid("app.fileExtensionOpeners", sourcePath: sourcePath) | ||
| return | ||
| } | ||
| normalized[normalizedExtension] = behavior.rawValue | ||
| } |
There was a problem hiding this comment.
cmux.json causes the return to exit the entire section-parsing function, so every setting that follows — openSupportedFilesInCmux, openMarkdownInCmuxViewer, iMessageMode, confirmQuit, and everything else in the app block — is silently dropped from the managed snapshot. The runtime reader (FileExtensionOpenBehaviorSettings.openers()) correctly uses continue to skip invalid entries; the file-store parser should do the same.
| for (rawExtension, rawBehavior) in values { | |
| guard let normalizedExtension = FileExtensionOpenBehavior.normalizedExtension(rawExtension), | |
| let behavior = FileExtensionOpenBehavior(rawValue: rawBehavior) else { | |
| logInvalid("app.fileExtensionOpeners", sourcePath: sourcePath) | |
| return | |
| } | |
| normalized[normalizedExtension] = behavior.rawValue | |
| } | |
| for (rawExtension, rawBehavior) in values { | |
| guard let normalizedExtension = FileExtensionOpenBehavior.normalizedExtension(rawExtension), | |
| let behavior = FileExtensionOpenBehavior(rawValue: rawBehavior) else { | |
| logInvalid("app.fileExtensionOpeners", sourcePath: sourcePath) | |
| continue | |
| } | |
| normalized[normalizedExtension] = behavior.rawValue | |
| } |
| @MainActor | ||
| static func deferredOpenCommandClickFile( | ||
| workspace: Workspace, | ||
| preferredWorkspaceId: UUID, | ||
| surfaceId: UUID, | ||
| filePath: String, | ||
| fallback: Fallback | ||
| ) { | ||
| DispatchQueue.main.async { | ||
| let resolvedWorkspace = AppDelegate.shared?.workspaceContainingPanel( | ||
| panelId: surfaceId, | ||
| preferredWorkspaceId: preferredWorkspaceId | ||
| )?.workspace ?? workspace | ||
| guard !resolvedWorkspace.isRemoteTerminalSurface(surfaceId) else { | ||
| fallback.open(URL(fileURLWithPath: filePath)) | ||
| return | ||
| } | ||
| guard openCommandClickFile( | ||
| workspace: resolvedWorkspace, | ||
| sourcePanelId: surfaceId, | ||
| filePath: filePath, | ||
| fallback: fallback | ||
| ) else { | ||
| fallback.open(URL(fileURLWithPath: filePath)) | ||
| return | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
deferredOpenCommandClickFile is already isolated to @MainActor, so DispatchQueue.main.async is a legacy dispatch that escapes Swift's actor model — the closure runs outside any actor isolation, dropping the @MainActor guarantee on the calls inside. Per the cmux-swift-concurrency-modernization rule, use Task { @MainActor in … } instead to keep the deferred work properly isolated.
| @MainActor | |
| static func deferredOpenCommandClickFile( | |
| workspace: Workspace, | |
| preferredWorkspaceId: UUID, | |
| surfaceId: UUID, | |
| filePath: String, | |
| fallback: Fallback | |
| ) { | |
| DispatchQueue.main.async { | |
| let resolvedWorkspace = AppDelegate.shared?.workspaceContainingPanel( | |
| panelId: surfaceId, | |
| preferredWorkspaceId: preferredWorkspaceId | |
| )?.workspace ?? workspace | |
| guard !resolvedWorkspace.isRemoteTerminalSurface(surfaceId) else { | |
| fallback.open(URL(fileURLWithPath: filePath)) | |
| return | |
| } | |
| guard openCommandClickFile( | |
| workspace: resolvedWorkspace, | |
| sourcePanelId: surfaceId, | |
| filePath: filePath, | |
| fallback: fallback | |
| ) else { | |
| fallback.open(URL(fileURLWithPath: filePath)) | |
| return | |
| } | |
| } | |
| } | |
| @MainActor | |
| static func deferredOpenCommandClickFile( | |
| workspace: Workspace, | |
| preferredWorkspaceId: UUID, | |
| surfaceId: UUID, | |
| filePath: String, | |
| fallback: Fallback | |
| ) { | |
| Task { @MainActor in | |
| let resolvedWorkspace = AppDelegate.shared?.workspaceContainingPanel( | |
| panelId: surfaceId, | |
| preferredWorkspaceId: preferredWorkspaceId | |
| )?.workspace ?? workspace | |
| guard !resolvedWorkspace.isRemoteTerminalSurface(surfaceId) else { | |
| fallback.open(URL(fileURLWithPath: filePath)) | |
| return | |
| } | |
| guard openCommandClickFile( | |
| workspace: resolvedWorkspace, | |
| sourcePanelId: surfaceId, | |
| filePath: filePath, | |
| fallback: fallback | |
| ) else { | |
| fallback.open(URL(fileURLWithPath: filePath)) | |
| return | |
| } | |
| } | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift`:
- Around line 108-117: The setBehavior(_:, for:) and remove(_:) methods
currently mutate the normalizedOpeners map using the raw fileExtension, causing
duplicate keys like ".html" vs "html" and failed removals; before writing or
removing, transform the incoming fileExtension to the same normalized form used
by normalizedOpeners (i.e., compute a normalizedKey from fileExtension using the
same normalization logic) and use that normalizedKey when setting
next[normalizedKey] = behavior and when calling next.removeValue(forKey:
normalizedKey) so openers stays consistent.
In `@Resources/Localizable.xcstrings`:
- Around line 143092-143107: The ja localization for the string key
settings.search.alias.setting.app.file-extension-openers currently duplicates
the English token list; replace that copied-English value with a proper Japanese
localization (update the "value" inside the "ja" -> "stringUnit" for
settings.search.alias.setting.app.file-extension-openers) using appropriate
Japanese terms for the tokens (e.g., localize phrases like "file extension
openers", "cmd click", "browser preview", "system default editor", etc.), keep
extractionState as "manual" and set the stringUnit state to "translated".
In `@Sources/CommandClickFileOpenRouter.swift`:
- Around line 246-253: Add a brief doc comment above the `@MainActor` static func
deferredOpenCommandClickFile(...) explaining that the function dispatches
asynchronously to avoid the Ghostty deadlock (same rationale as
deferredOpenFileInCmux), summarizing the deadlock/workaround and referencing
that it mirrors deferredOpenFileInCmux’s behavior so maintainers know why the
async dispatch is necessary.
🪄 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: 4906919c-6175-467a-8fe3-6863f09d8bae
📒 Files selected for processing (21)
Packages/CmuxSettings/Sources/CmuxSettings/Keys/AppCatalogSection.swiftPackages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehavior.swiftPackages/CmuxSettings/Tests/CmuxSettingsTests/SettingCodableTests.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/SettingsSectionID.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swiftPackages/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/CmuxSettingsJSONPathSupport.swiftSources/CommandClickFileOpenRouter.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/WindowAndDragTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.json
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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
`@Packages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swift`:
- Around line 8-46: The methods openers, behavior(forPath:), setOpeners, and
notifyDidChange currently default to UserDefaults.standard and
NotificationCenter.default; remove those default parameter values so callers
must inject UserDefaults and NotificationCenter. Specifically, change
openers(defaults: UserDefaults) -> [String: FileExtensionOpenBehavior] and
behavior(forPath: String, defaults: UserDefaults) -> FileExtensionOpenBehavior?
so they no longer use .standard, and change setOpeners(_ openers: [String:
FileExtensionOpenBehavior], defaults: UserDefaults, notificationCenter:
NotificationCenter) and notifyDidChange(notificationCenter: NotificationCenter)
to require explicit instances; update internal calls (e.g., behavior calling
openers) to forward the passed defaults/notificationCenter and update all call
sites to pass the appropriate injected instances. Ensure normalized logic and
return types remain unchanged.
- Around line 8-21: The openers(defaults:) function currently builds an empty
result so any persisted override removes all built-in defaults; change the logic
in openers to seed result from FileExtensionOpenBehavior.defaultValue (rather
than starting empty) and then iterate stored to normalize keys (using
FileExtensionOpenBehavior.normalizedExtension(_:)) and override matching entries
with parsed behaviors (FileExtensionOpenBehavior(rawValue:)), so defaults remain
unless explicitly replaced; update references to result, normalizedExtension,
and behavior accordingly.
- Around line 3-49: Add Swift-DocC triple-slash (///) comments for the new
public API introduced in the FileExtensionOpenBehaviorSettings enum: document
the enum itself and each public member including key, didChangeNotification,
defaultValue, and the methods openers(defaults:), behavior(forPath:defaults:),
setOpeners(_:defaults:notificationCenter:), and
notifyDidChange(notificationCenter:). For each symbol include a concise one-line
summary of purpose and any important parameter/return semantics (e.g., that
openers reads normalized mappings from UserDefaults and setOpeners writes
normalized rawValue strings and posts didChangeNotification). Ensure comments
are placed immediately above each declaration using /// so the package surface
is documented.
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry`+Default.swift:
- Around line 59-64: Replace the hard-coded English titles in the
CuratedSettingEntry initializers (the .init calls with section: .fileOpening and
ids "file-drops", "preferred-editor", "file-extension-openers",
"supported-file-previews", "markdown-viewer") with localized strings using the
Swift localization API (e.g., String(localized: , defaultValue: ) or your
project's equivalent) and reference matching keys in the string catalog; ensure
each title uses a unique localization key and add corresponding translations to
the .strings or .stringsdict entries so the settings-search UI shows localized
text.
In `@Sources/cmuxApp.swift`:
- Around line 6918-6925: Update the subtitle for the SettingsCardRow that uses
configurationReview .json("app.preferredEditor") by replacing the localized
string key "settings.app.preferredEditor.subtitle" with copy that explains the
new opener flow (it should mention both the fallback and the explicit routing of
extensions to the preferred editor); update the String(localized: ...) call in
the SettingsCardRow initializer (the one adjacent to TextField and
preferredEditorCommand) to reference the new copy and then add corresponding
translated entries for that same key in all Resources/*.xcstrings files so every
supported locale is updated.
🪄 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: 5ea31bdf-a05d-4f22-a015-d50fbc4a5348
📒 Files selected for processing (12)
Packages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/SettingsSectionID.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/AppSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swiftPackages/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/CommandClickFileOpenRouter.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/cmuxApp.swift
💤 Files with no reviewable changes (1)
- Sources/CommandClickFileOpenRouter.swift
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift`:
- Around line 17-18: The current FileExtensionOpenersEditor recomputes let
extensions = sortedExtensions by calling openers.keys.sorted(...) on every
render which still sorts the entire collection; change this to cache a sorted
snapshot whenever the source map (app.fileExtensionOpeners / openers) changes
and then use a capped slice of that cached array in body rendering. Concretely,
add a stored `@State` or `@StateObject` (e.g., cachedSortedExtensions) and update it
only from the mutating path or in an onChange(of: openers) handler or a computed
property that compares a version/identity token; replace direct calls to
openers.keys.sorted(...) and usages of sortedExtensions with the
cachedSortedExtensions and then take prefix(32) for UI rows so the heavy sort
runs only when openers actually changes. Ensure the change covers the other
spots you flagged (the uses around lines 44-56 and 71-76) by reading from the
same cachedSortedExtensions.
🪄 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: eebcc2a6-0fe2-4105-ab45-8ead71d26ec6
📒 Files selected for processing (4)
Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swiftResources/Localizable.xcstringsSources/SettingsNavigation.swiftcmuxTests/WorkspaceUnitTests.swift
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| } | ||
| result[normalizedExtension] = behavior | ||
| } | ||
| return result |
There was a problem hiding this comment.
Built-in defaults block removal
Medium Severity
FileExtensionOpenBehaviorSettings.openers always re-injects built-in html/htm browser entries on read, but setOpeners and DefaultsValueModel persist only explicit overrides. Removing those rows writes a map without them, yet Cmd-click routing still uses the browser. The legacy card reloads via openers() after didChangeNotification, so removed built-ins can reappear immediately while behavior never matches the edited list.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 559c9f2. Configure here.
| fallback?() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Unused deferred file open helpers
Low Severity
This commit switches terminal link handling to deferredOpenCommandClickFile, but deferredOpenFileInCmux and its openInCmux call remain with no remaining callers. That dead path duplicates extension-aware opening logic and can mislead future changes back to the old deferral behavior.
Reviewed by Cursor Bugbot for commit 559c9f2. Configure here.
| var next = normalizedOpenersCache | ||
| next[normalized] = behavior | ||
| applyOpeners(next) | ||
| } | ||
|
|
||
| private func remove(_ fileExtension: String) { | ||
| guard let normalized = FileExtensionOpenBehavior.normalizedExtension(fileExtension) else { return } |
There was a problem hiding this comment.
remove("html") writes a map without "html" to UserDefaults, but FileExtensionOpenBehaviorSettings.openers() always starts from defaultValue and merges stored entries on top — so "html" is immediately re-inserted with .cmuxBrowser the moment UserDefaults.didChangeNotification fires in FileExtensionOpenersSettingsCard. The user sees the row flash away and reappear. Worse, if the user had previously overridden html to .preferredEditor and then presses minus expecting to reset to the global toggle, their stored override is erased and the built-in default .cmuxBrowser is silently restored instead.
The fix is to either: (a) hide or replace the minus button with a "Reset" button for entries whose normalized key is in FileExtensionOpenBehavior.defaultOpeners, making the intent explicit; or (b) make "remove" write .automatic for default-keyed entries so the stored entry wins over the built-in default in the merge.
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 (7)
Packages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swift (1)
21-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the tolerant enum decoder here.
Unknown persisted values are currently skipped outright, so a stale/corrupt entry for
"html"will silently re-enable the built-in.cmuxBrowserdefault instead of degrading to.automatic. That diverges from the newFileExtensionOpenBehavior.decodeFromUserDefaults(_:)behavior.Suggested fix
for (rawExtension, rawBehavior) in stored { guard let normalizedExtension = FileExtensionOpenBehavior.normalizedExtension(rawExtension), - let rawBehavior = rawBehavior as? String, - let behavior = FileExtensionOpenBehavior(rawValue: rawBehavior) else { + let behavior = FileExtensionOpenBehavior.decodeFromUserDefaults(rawBehavior) else { continue } result[normalizedExtension] = behavior }🤖 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/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swift` around lines 21 - 24, The loop currently uses FileExtensionOpenBehavior.normalizedExtension(...) and FileExtensionOpenBehavior(rawValue: rawBehavior) which silently drops unknown persisted values; replace the direct enum initializer with the tolerant decoder by calling FileExtensionOpenBehavior.decodeFromUserDefaults(rawBehavior) (or the equivalent tolerant decode method) so unknown/stale values degrade to .automatic instead of being skipped—keep the normalizedExtension check and use the decoder's result to produce the behavior used in the mapping.Sources/CommandClickFileOpenRouter.swift (1)
211-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep Ghostty deadlock deferral on
DispatchQueue.main.asyncindeferredOpenCommandClickFile
deferredOpenCommandClickFilesays it mirrorsdeferredOpenFileInCmux(which documents deferring viaDispatchQueue.main.asyncto avoid theSurface.openUrlre-entry/deadlock), but the implementation schedules viaTask {@mainactorin ... }instead.Task {@mainactorin }guarantees MainActor isolation, not an explicit “next run-loop tick” deferral contract, so this can reintroduce the risk the workaround is meant to prevent.Suggested fix
- Task { `@MainActor` in + DispatchQueue.main.async { let resolvedWorkspace = AppDelegate.shared?.workspaceContainingPanel( panelId: surfaceId, preferredWorkspaceId: preferredWorkspaceId )?.workspace ?? workspace guard !resolvedWorkspace.isRemoteTerminalSurface(surfaceId) else { fallback.open(URL(fileURLWithPath: filePath)) return } guard openCommandClickFile( workspace: resolvedWorkspace, sourcePanelId: surfaceId, filePath: filePath, fallback: fallback ) else { fallback.open(URL(fileURLWithPath: filePath)) return } - } + }🤖 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/CommandClickFileOpenRouter.swift` around lines 211 - 229, The current deferredOpenCommandClickFile implementation uses Task { `@MainActor` in ... } which only enforces MainActor isolation but does not guarantee a run-loop tick deferral; change the scheduling to DispatchQueue.main.async { ... } to mirror deferredOpenFileInCmux and avoid Surface.openUrl re-entrancy/deadlock. Locate the block inside deferredOpenCommandClickFile that resolves resolvedWorkspace (using AppDelegate.shared?.workspaceContainingPanel and isRemoteTerminalSurface) and that calls openCommandClickFile/fallback.open, and replace the Task { `@MainActor` in ... } scheduling with DispatchQueue.main.async { ... } so the body runs on the next main run-loop tick while still performing the same workspace checks and fallback logic.Sources/KeyboardShortcutSettingsFileStore.swift (1)
436-446:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize opener values when loading
app.fileExtensionOpeners.This path normalizes extensions but still parses behaviors with
FileExtensionOpenBehavior(rawValue:), so it only accepts exact enum spellings. That bypasses the normalization/decoding behavior introduced for this setting and makescmux.jsonstricter than the rest of the feature contract. Please parserawBehaviorthrough the sharedFileExtensionOpenBehaviornormalization helper before persisting the dictionary.🤖 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/KeyboardShortcutSettingsFileStore.swift` around lines 436 - 446, The code normalizes file extensions but still constructs behaviors with FileExtensionOpenBehavior(rawValue:), rejecting non-exact spellings; change the parsing to use the shared FileExtensionOpenBehavior normalization helper (the helper on the FileExtensionOpenBehavior type) to convert rawBehavior into a normalized enum case before using behavior.rawValue when populating normalized; keep the existing guard/logInvalid/continue flow so failures still log via logInvalid("app.fileExtensionOpeners", sourcePath: sourcePath) and then assign snapshot.managedUserDefaults[FileExtensionOpenBehaviorSettings.key] = .stringDictionary(normalized).Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swift (2)
5-25: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDocument the new public package API.
FileOpeningSection, its public initializer, and its publicbodyare new public package symbols, but none of them have DocC comments yet.As per coding guidelines,
Packages/**/*.swift:Every public symbol in any new Swift package under Packages/ is documented with a Swift-DocC triple-slash comment (///) at the time of writing.🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swift` around lines 5 - 25, Add Swift-DocC triple-slash comments for the new public API: document the FileOpeningSection type, its public init(defaultsStore:catalog:) and the public body property. For FileOpeningSection add a top-level /// comment describing the purpose of the view; for the init add /// lines describing the parameters (defaultsStore and catalog) and what the initializer configures; and for the public var body add a brief /// describing the view’s content/behavior. Place the comments immediately above the struct declaration, the init signature, and the body property respectively so the public symbols are fully documented.
53-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWire row anchors for the new file-opening settings.
SettingsNavigation.swiftregisters row-level anchors forapp.preferredEditor,app.openSupportedFilesInCmux, andapp.openMarkdownInCmuxViewer, but these rows never publish matchingsearchAnchorIDs. Search results for those settings will not jump to or highlight the exact control.♻️ Suggested change
SettingsCardRow( configurationReview: .json("app.preferredEditor"), + searchAnchorID: "setting:fileOpening:preferred-editor", String(localized: "settings.app.preferredEditor", defaultValue: "Open Files With"), subtitle: String(localized: "settings.app.preferredEditor.subtitle", defaultValue: "Command used when an extension is set to Preferred Editor, or when Cmd-click file previews are disabled or a file is unsupported. Leave empty for system default.") ) { @@ SettingsCardRow( configurationReview: .json("app.openSupportedFilesInCmux"), + searchAnchorID: "setting:fileOpening:supported-file-previews", String(localized: "settings.app.openSupportedFilesInCmux", defaultValue: "Open Supported Files in cmux"), subtitle: String(localized: "settings.app.openSupportedFilesInCmux.subtitle", defaultValue: "Cmd-clicking readable files opens text, code, PDFs, images, audio, video, and Quick Look previews in cmux.") ) { @@ SettingsCardRow( configurationReview: .json("app.openMarkdownInCmuxViewer"), + searchAnchorID: "setting:fileOpening:markdown-viewer", String(localized: "settings.app.openMarkdownInCmuxViewer", defaultValue: "Open Markdown in cmux Viewer"), subtitle: String(localized: "settings.app.openMarkdownInCmuxViewer.subtitle", defaultValue: "When supported file routing is on, Cmd-clicking Markdown files opens the rendered cmux markdown viewer instead of the generic file preview.") ) {Based on learnings, when adding a new Settings section in SwiftUI, navigation/search wiring needs matching entries and anchors so jump-to behavior does not break.
Also applies to: 77-97
🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swift` around lines 53 - 64, The SettingsCardRow entries for the new file-opening controls (the SettingsCardRow with configurationReview .json("app.preferredEditor") and the other two rows for "app.openSupportedFilesInCmux" and "app.openMarkdownInCmuxViewer") need to publish matching search anchor IDs so navigation/search can jump to the exact control; add a search anchor/anchor preference to each row (or the control inside it, e.g., the TextField or toggle) that emits the same identifier used in SettingsNavigation.swift (app.preferredEditor, app.openSupportedFilesInCmux, app.openMarkdownInCmuxViewer) so the existing SearchAnchorKey/lookup can find and highlight the element. Ensure the anchor is attached to the interactive control (TextField or Toggle) and uses the exact string keys registered in SettingsNavigation.swift.Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift (2)
5-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDocument the new public package API.
FileExtensionOpenersEditor, its public initializer, and its publicbodywere added without DocC comments. Please add///summaries so the package surface complies with the package API documentation rule.As per coding guidelines,
Packages/**/*.swift:Every public symbol in any new Swift package under Packages/ is documented with a Swift-DocC triple-slash comment (///) at the time of writing.🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift` around lines 5 - 21, Add Swift-DocC triple-slash comments for the new public API: document the public struct FileExtensionOpenersEditor with a one-line summary of its purpose, document the public init(openers:) with a brief description of the parameter and initialization behavior, and document the public body property with a short description of the view it produces; place /// comments immediately above the declarations of FileExtensionOpenersEditor, init(openers: Binding<[String: FileExtensionOpenBehavior]>), and the body property so the package meets the public-symbol documentation rule.
25-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the missing anchor for the “File Extension Openers” row.
SettingsSearchIndexnow mapsapp.fileExtensionOpenerstosetting:fileOpening:file-extension-openers, but this row never exposes that anchor. Search/jump-to will find the entry, then fall back to the section instead of scrolling to and highlighting the editor.♻️ Suggested change
SettingsCardRow( configurationReview: .json("app.fileExtensionOpeners"), + searchAnchorID: "setting:fileOpening:file-extension-openers", String(localized: "settings.app.fileExtensionOpeners", defaultValue: "File Extension Openers"), subtitle: String(localized: "settings.app.fileExtensionOpeners.subtitle", defaultValue: "Add any file extension and choose whether Cmd-click opens it in preview, browser, Markdown viewer, preferred editor, or system default. HTML opens in the cmux browser by default.") ) {Based on learnings, when adding a new Settings section in SwiftUI, navigation/search wiring needs matching entries and anchors so jump-to behavior does not break.
🤖 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/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift` around lines 25 - 29, The SettingsCardRow for the "File Extension Openers" entry is missing the anchor that SettingsSearchIndex maps to (app.fileExtensionOpeners -> setting:fileOpening:file-extension-openers), so searches jump to the section instead of the row; fix this by exposing the matching anchor on the SettingsCardRow (the same view that uses configurationReview: .json("app.fileExtensionOpeners"))—e.g., add the anchor/id/accessibility identifier "setting:fileOpening:file-extension-openers" to that SettingsCardRow so jump-to highlights and scrolls to the editor.
🤖 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/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swift`:
- Around line 21-24: The loop currently uses
FileExtensionOpenBehavior.normalizedExtension(...) and
FileExtensionOpenBehavior(rawValue: rawBehavior) which silently drops unknown
persisted values; replace the direct enum initializer with the tolerant decoder
by calling FileExtensionOpenBehavior.decodeFromUserDefaults(rawBehavior) (or the
equivalent tolerant decode method) so unknown/stale values degrade to .automatic
instead of being skipped—keep the normalizedExtension check and use the
decoder's result to produce the behavior used in the mapping.
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swift`:
- Around line 5-21: Add Swift-DocC triple-slash comments for the new public API:
document the public struct FileExtensionOpenersEditor with a one-line summary of
its purpose, document the public init(openers:) with a brief description of the
parameter and initialization behavior, and document the public body property
with a short description of the view it produces; place /// comments immediately
above the declarations of FileExtensionOpenersEditor, init(openers:
Binding<[String: FileExtensionOpenBehavior]>), and the body property so the
package meets the public-symbol documentation rule.
- Around line 25-29: The SettingsCardRow for the "File Extension Openers" entry
is missing the anchor that SettingsSearchIndex maps to (app.fileExtensionOpeners
-> setting:fileOpening:file-extension-openers), so searches jump to the section
instead of the row; fix this by exposing the matching anchor on the
SettingsCardRow (the same view that uses configurationReview:
.json("app.fileExtensionOpeners"))—e.g., add the anchor/id/accessibility
identifier "setting:fileOpening:file-extension-openers" to that SettingsCardRow
so jump-to highlights and scrolls to the editor.
In
`@Packages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swift`:
- Around line 5-25: Add Swift-DocC triple-slash comments for the new public API:
document the FileOpeningSection type, its public init(defaultsStore:catalog:)
and the public body property. For FileOpeningSection add a top-level /// comment
describing the purpose of the view; for the init add /// lines describing the
parameters (defaultsStore and catalog) and what the initializer configures; and
for the public var body add a brief /// describing the view’s content/behavior.
Place the comments immediately above the struct declaration, the init signature,
and the body property respectively so the public symbols are fully documented.
- Around line 53-64: The SettingsCardRow entries for the new file-opening
controls (the SettingsCardRow with configurationReview
.json("app.preferredEditor") and the other two rows for
"app.openSupportedFilesInCmux" and "app.openMarkdownInCmuxViewer") need to
publish matching search anchor IDs so navigation/search can jump to the exact
control; add a search anchor/anchor preference to each row (or the control
inside it, e.g., the TextField or toggle) that emits the same identifier used in
SettingsNavigation.swift (app.preferredEditor, app.openSupportedFilesInCmux,
app.openMarkdownInCmuxViewer) so the existing SearchAnchorKey/lookup can find
and highlight the element. Ensure the anchor is attached to the interactive
control (TextField or Toggle) and uses the exact string keys registered in
SettingsNavigation.swift.
In `@Sources/CommandClickFileOpenRouter.swift`:
- Around line 211-229: The current deferredOpenCommandClickFile implementation
uses Task { `@MainActor` in ... } which only enforces MainActor isolation but does
not guarantee a run-loop tick deferral; change the scheduling to
DispatchQueue.main.async { ... } to mirror deferredOpenFileInCmux and avoid
Surface.openUrl re-entrancy/deadlock. Locate the block inside
deferredOpenCommandClickFile that resolves resolvedWorkspace (using
AppDelegate.shared?.workspaceContainingPanel and isRemoteTerminalSurface) and
that calls openCommandClickFile/fallback.open, and replace the Task { `@MainActor`
in ... } scheduling with DispatchQueue.main.async { ... } so the body runs on
the next main run-loop tick while still performing the same workspace checks and
fallback logic.
In `@Sources/KeyboardShortcutSettingsFileStore.swift`:
- Around line 436-446: The code normalizes file extensions but still constructs
behaviors with FileExtensionOpenBehavior(rawValue:), rejecting non-exact
spellings; change the parsing to use the shared FileExtensionOpenBehavior
normalization helper (the helper on the FileExtensionOpenBehavior type) to
convert rawBehavior into a normalized enum case before using behavior.rawValue
when populating normalized; keep the existing guard/logInvalid/continue flow so
failures still log via logInvalid("app.fileExtensionOpeners", sourcePath:
sourcePath) and then assign
snapshot.managedUserDefaults[FileExtensionOpenBehaviorSettings.key] =
.stringDictionary(normalized).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 131a32e7-0ff0-4b4e-93a7-4cfdc23da85c
📒 Files selected for processing (13)
Packages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehavior.swiftPackages/CmuxSettings/Sources/CmuxSettings/Values/FileExtensionOpenBehaviorSettings.swiftPackages/CmuxSettings/Tests/CmuxSettingsTests/SettingCodableTests.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Rows/FileExtensionOpenersEditor.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/FileOpeningSection.swiftResources/Localizable.xcstringsSources/CommandClickFileOpenRouter.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/SettingsNavigation.swiftSources/cmuxApp.swiftcmuxTests/WindowAndDragTests.swiftcmuxTests/WorkspaceUnitTests.swift
| guard let normalizedExtension = FileExtensionOpenBehavior.normalizedExtension(ext) else { | ||
| return nil | ||
| } | ||
| return openers(defaults: defaults)[normalizedExtension] |
There was a problem hiding this comment.
Settings and routing disagree
Medium Severity
Cmd-click routing reads fileExtensionOpeners through FileExtensionOpenBehaviorSettings.openers, which always merges built-in entries such as html/htm → cmuxBrowser. The File Opening settings UI persists and reloads the raw UserDefaults map via DefaultsValueModel, so partial saves, an empty {}, or removing rows can show “no overrides” while HTML still opens in the cmux browser. The legacy card reload path re-merges defaults and can make removed built-in rows reappear.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f864d02. Configure here.
| var next = normalizedOpenersCache | ||
| if next[normalized] == nil { | ||
| next[normalized] = .cmuxPreview | ||
| } |
There was a problem hiding this comment.
New extensions default preview
Low Severity
When adding an extension in File Extension Openers, a new key is always initialized to cmuxPreview. Re-adding html after it is absent from the stored map therefore opens Cmd-click preview instead of the product default cmuxBrowser used by routing until the user changes the picker.
Reviewed by Cursor Bugbot for commit f864d02. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f864d02c39
ℹ️ 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".
| private func remove(_ fileExtension: String) { | ||
| guard let normalized = FileExtensionOpenBehavior.normalizedExtension(fileExtension) else { return } | ||
| var next = normalizedOpenersCache | ||
| next.removeValue(forKey: normalized) |
There was a problem hiding this comment.
When the user removes one of the built-in rows such as .html or .htm, this only deletes the key from the stored map. FileExtensionOpenBehaviorSettings.openers(defaults:) then starts from defaultValue and overlays the stored entries, so the deleted built-in key is silently reintroduced for routing while the Settings UI no longer shows it. In that scenario Cmd-click still opens HTML in the cmux browser even though the user just removed the opener; removal needs to write an explicit override (or make built-in rows reset/non-removable) instead of simply omitting the key.
Useful? React with 👍 / 👎.
| if CmdClickSupportedFileRouteSettings.shouldRoute(path: path, defaults: defaults) { | ||
| return .cmuxPreview | ||
| } | ||
| return .unhandled |
There was a problem hiding this comment.
Automatic HTML still routes cmux
Medium Severity
Choosing automatic for built-in extensions such as html is meant to opt out of the built-in opener, but routing still treats the file like a normal supported preview. Cmd-click on .html can open in cmux preview instead of staying external, which conflicts with the documented opt-out and the new routing tests.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f0e9b09. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0e9b0954c
ℹ️ 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".
| case .automatic: | ||
| break |
There was a problem hiding this comment.
When a user sets a built-in opener such as html to automatic to opt out of the new cmux-browser default, this break falls through to the legacy Markdown/supported-file checks. For a readable .html file with the default openSupportedFilesInCmux setting still enabled, CmdClickSupportedFileRouteSettings.shouldRoute also falls through on .automatic, so shouldHandleCommandClick returns true and Cmd-click opens a cmux preview instead of no longer being handled by cmux. This is the exact scenario documented for disabling built-in defaults, so the automatic override needs to suppress the built-in route rather than re-enter generic supported-file routing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1f1ab68. Configure here.
| case .markdownViewer, .cmuxBrowser, .preferredEditor, .systemDefault: | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Automatic HTML still previews
Medium Severity
Setting html or htm to automatic is meant to opt out of the built-in browser opener, but CmdClickSupportedFileRouteSettings.shouldRoute still treats those paths like any other extension after the automatic branch and can route readable HTML into cmux preview when openSupportedFilesInCmux is on. That conflicts with the new tests and with keeping HTML off the generic supported-file path.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1f1ab68. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1ab68bff
ℹ️ 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".
| ) { | ||
| NSWorkspace.shared.open(fileURL) | ||
| } | ||
| filePath: fileURL.path, |
There was a problem hiding this comment.
When Cmd-clicking a local file URL that includes a fragment or query and the extension is handled by the new .cmuxBrowser route (HTML by default), this passes only fileURL.path into the router. openCommandClickFile then reconstructs URL(fileURLWithPath:), so links such as file:///tmp/report.html#section open file:///tmp/report.html in the cmux browser and lose the anchor/query that the terminal link originally contained. This only affects routed file URLs; pass the original target.url through the browser/system-default path or preserve its components before stripping for preview-only routes.
Useful? React with 👍 / 👎.
| private func jsonStringDictionary(_ rawValue: Any?) -> [String: String]? { | ||
| guard let values = rawValue as? [String: Any] else { return nil } | ||
| var strings: [String: String] = [:] | ||
| strings.reserveCapacity(values.count) | ||
| for (key, value) in values { | ||
| guard let string = value as? String else { return nil } | ||
| strings[key] = string | ||
| } | ||
| return strings | ||
| } |
There was a problem hiding this comment.
"pdf": 123) causes jsonStringDictionary to return nil for the entire dict, so the caller's outer if let values = ... branch never runs and all valid extension openers are silently discarded — only logInvalid is called. The runtime FileExtensionOpenBehaviorSettings.openers() uses continue to skip bad entries; the file-store parser should match that behaviour by skipping non-string values rather than bailing out of the whole dictionary.
| private func jsonStringDictionary(_ rawValue: Any?) -> [String: String]? { | |
| guard let values = rawValue as? [String: Any] else { return nil } | |
| var strings: [String: String] = [:] | |
| strings.reserveCapacity(values.count) | |
| for (key, value) in values { | |
| guard let string = value as? String else { return nil } | |
| strings[key] = string | |
| } | |
| return strings | |
| } | |
| private func jsonStringDictionary(_ rawValue: Any?) -> [String: String]? { | |
| guard let values = rawValue as? [String: Any] else { return nil } | |
| var strings: [String: String] = [:] | |
| strings.reserveCapacity(values.count) | |
| for (key, value) in values { | |
| guard let string = value as? String else { continue } | |
| strings[key] = string | |
| } | |
| return strings | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c433259ab
ℹ️ 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".
| import SwiftUI | ||
|
|
||
| @MainActor | ||
| public struct FileOpeningSection: View { |
There was a problem hiding this comment.
The repo's AGENTS.md requires every public symbol added under Packages/ to have Swift-DocC /// documentation. This new public view API (and its public initializer/body below) is currently undocumented, so the package docs contract is still violated even though the earlier FileExtensionOpenBehavior docs have been added; please add DocC comments for the public symbols introduced here.
Useful? React with 👍 / 👎.
Addressed in later commits on this PR; the latest CodeRabbit check on head 4297516 passed/skipped.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Summary
.htmland.htmpaths to the cmux browser by default instead of the file preview editor.cmux.json, with browser, preview, Markdown, preferred editor, system default, and automatic behaviors.app.fileExtensionOpenersconfig schema.Testing
swift test --package-path Packages/CmuxSettings --filter SettingCodableswift test --package-path Packages/CmuxSettingsUIbun testinweb/bun run lintinweb/(passes with existing warnings)./scripts/reload.sh --tag fileopen --swift-frontend-workaroundNote: full
swift test --package-path Packages/CmuxSettingsstill hits the existingSettingCatalogTests.userDefaultsStorageKeysAreUniquefailure onmain(115 keys vs 106 unique keys onmain; 116 vs 107 with this branch).Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes default Cmd-click behavior for HTML and centralizes file-open routing; scope is settings and UX rather than auth or data, with tests covering overrides and JSON loading.
Overview
Adds per-extension Cmd-click routing via
app.fileExtensionOpeners, with a dedicated File Opening settings section andcmux.jsonsupport.Defaults and behavior:
.html/.htmnow open in the cmux browser instead of the generic file preview. Each extension can be set to automatic (legacy Markdown/supported-file toggles), cmux preview, Markdown viewer, browser, preferred editor, or system default. Built-in defaults merge with user overrides; storingautomaticon a built-in extension opts out of that default.Routing:
CommandClickFileOpenRouterresolves extension rules first, then existing settings. Terminal link handling uses deferred opens with fallbacks; browser routes keep URL fragments when opening local files.UI / config: File drops, preferred editor, extension editor (add/remove, show-all cap), and related toggles move out of App into File Opening. Search anchors, localization, schema docs, and tests cover storage, JSON import, and HTML routing regressions.
Reviewed by Cursor Bugbot for commit 4297516. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Cmd-click on
.html/.htmnow opens in the cmux browser by default. File opening is configurable per extension via Settings andcmux.json, with a new “File Opening” section and a router that falls back to your preferred editor or the system default.New Features
app.fileExtensionOpenersmaps extensions to behaviors: automatic, cmuxPreview, markdownViewer, cmuxBrowser, preferredEditor, systemDefault. Defaults.html/.htm→cmuxBrowser; keys may include or omit a leading dot; values are normalized; built-in defaults are merged and can be overridden. Changes post a notification and the Settings UI uses observer tokens to stay in sync.shouldHandleCommandClickresolves extension rules first, then legacy Markdown/supported-file toggles. “automatic” respects built‑in defaults (so HTML won’t route to preview). Opens/focuses a browser split for local HTML, and falls back to preferred editor or system default if in‑app open fails; terminal link handling uses this path.cmux.jsonschema/loader validate, normalize, and merge openers with defaults; the template seeds defaults; tests cover defaults, overrides, JSON parsing, routing, and an HTML preview regression. Configuration docs now include the File Opening section and an example forfileExtensionOpeners.Migration
cmux.json, for example:"fileExtensionOpeners": { "html": "cmuxPreview" }"fileExtensionOpeners": { "html": "systemDefault" }"fileExtensionOpeners": { "html": "preferredEditor" }Written for commit 4297516. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation