Repository navigation
Add React Feed to the workspace menu - #8171
lawrencecchen wants to merge 29 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds a native-backed Feed webview with React UI, WebKit bridge messaging, bundled compressed assets, feed actions in application menus, feed state observation, browser capability propagation, and omnibar visibility persistence. ChangesFeed surface
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPalette
participant AppDelegate
participant BrowserPanel
participant FeedSurfaceBridge
participant FeedApp
User->>CommandPalette: Select Feed
CommandPalette->>AppDelegate: Execute feed action
AppDelegate->>BrowserPanel: Open feed workspace surface
BrowserPanel->>FeedSurfaceBridge: Install bridge and serve feed assets
FeedApp->>FeedSurfaceBridge: Subscribe to feed snapshots
FeedSurfaceBridge-->>FeedApp: Publish feed.snapshot
User->>FeedApp: Choose permission action
FeedApp->>FeedSurfaceBridge: Submit permission reply
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Greptile SummaryThis PR adds a React Feed surface accessible from the workspace titlebar dropdown and command palette, backed by a new
Confidence Score: 4/5Safe to merge after addressing the overly broad UserDefaults observer in FeedSurfaceBridge. The settings observer in FeedSurfaceBridge subscribes to all UserDefaults changes rather than only the four keys that actually affect integration readiness. Every unrelated settings write cancels the in-flight filesystem scan, resets all integrations to Checking, publishes that incorrect state to the subscribed Feed webview, and restarts probing across 9 config files. The rest of the PR is solid. Sources/Feed/FeedSurfaceBridge.swift — the settingsObserver subscription scope. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant AppDelegate
participant BrowserPanel
participant FeedSurfaceBridge
participant FeedCoordinator
participant ReactFeedApp
User->>AppDelegate: Click Feed menu or palette
AppDelegate->>FeedSurfaceBridge: feedURL() registers bundled assets
FeedSurfaceBridge-->>AppDelegate: cmux-diff-viewer://bundled-feed-surface/feed.html
AppDelegate->>BrowserPanel: init nativeCapabilities .feed
BrowserPanel->>FeedSurfaceBridge: installIfNeeded on userContentController
ReactFeedApp->>FeedSurfaceBridge: postMessage feed.subscribe
FeedSurfaceBridge->>FeedCoordinator: read store items
FeedSurfaceBridge-->>ReactFeedApp: initial snapshot
FeedSurfaceBridge->>FeedSurfaceBridge: armSnapshotSubscription withObservationTracking
FeedCoordinator->>FeedSurfaceBridge: onChange items mutated
FeedSurfaceBridge->>ReactFeedApp: evaluateJavaScript cmuxFeedBridge.receive snapshot
ReactFeedApp->>FeedSurfaceBridge: postMessage feed.permission.reply
FeedSurfaceBridge->>FeedCoordinator: deliverReply decision permission mode
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant AppDelegate
participant BrowserPanel
participant FeedSurfaceBridge
participant FeedCoordinator
participant ReactFeedApp
User->>AppDelegate: Click Feed menu or palette
AppDelegate->>FeedSurfaceBridge: feedURL() registers bundled assets
FeedSurfaceBridge-->>AppDelegate: cmux-diff-viewer://bundled-feed-surface/feed.html
AppDelegate->>BrowserPanel: init nativeCapabilities .feed
BrowserPanel->>FeedSurfaceBridge: installIfNeeded on userContentController
ReactFeedApp->>FeedSurfaceBridge: postMessage feed.subscribe
FeedSurfaceBridge->>FeedCoordinator: read store items
FeedSurfaceBridge-->>ReactFeedApp: initial snapshot
FeedSurfaceBridge->>FeedSurfaceBridge: armSnapshotSubscription withObservationTracking
FeedCoordinator->>FeedSurfaceBridge: onChange items mutated
FeedSurfaceBridge->>ReactFeedApp: evaluateJavaScript cmuxFeedBridge.receive snapshot
ReactFeedApp->>FeedSurfaceBridge: postMessage feed.permission.reply
FeedSurfaceBridge->>FeedCoordinator: deliverReply decision permission mode
Reviews (15): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static func feedURL() -> URL? { | ||
| do { | ||
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | ||
| } catch { | ||
| NSLog("feed.surface.register.failed error=%@", String(describing: error)) | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
NSLog bypasses the unified logging subsystem. The repo's logging rule requires os.Logger for all production diagnostics; NSLog output is not captured by the unified log store and cannot be filtered by subsystem or category in Console.app or log stream.
| static func feedURL() -> URL? { | |
| do { | |
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | |
| } catch { | |
| NSLog("feed.surface.register.failed error=%@", String(describing: error)) | |
| return nil | |
| } | |
| } | |
| static func feedURL() -> URL? { | |
| do { | |
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | |
| } catch { | |
| Logger.feed.error("feed.surface.register.failed error=\(error, privacy: .public)") | |
| return nil | |
| } | |
| } |
Rule Used: Flag production Swift diagnostics that bypass unif... (source)
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!
| static func isTrustedFeedURL(_ url: URL?, resourceURL: URL? = Bundle.main.resourceURL) -> Bool { | ||
| _ = resourceURL | ||
| guard let url, | ||
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | ||
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | ||
| requestPath: "/feed.html" | ||
| ) else { return false } | ||
| return url == expected | ||
| } |
There was a problem hiding this comment.
resourceURL parameter is accepted with a meaningful default but immediately discarded via _ = resourceURL. The function derives its trust anchor solely from CmuxDiffViewerURLSchemeHandler.diffViewerURL, making the parameter a no-op. This is misleading to callers who may expect passing a different bundle root to affect the result.
| static func isTrustedFeedURL(_ url: URL?, resourceURL: URL? = Bundle.main.resourceURL) -> Bool { | |
| _ = resourceURL | |
| guard let url, | |
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | |
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | |
| requestPath: "/feed.html" | |
| ) else { return false } | |
| return url == expected | |
| } | |
| static func isTrustedFeedURL(_ url: URL?) -> Bool { | |
| guard let url, | |
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | |
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | |
| requestPath: "/feed.html" | |
| ) else { return false } | |
| return url == expected | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/AppDelegate`+NewWorkspaceContextMenu.swift:
- Around line 120-133: Extract the shared configured-action gating from
resolvedBuiltInFeedAction and shouldAppendBuiltInNewAgentChatMenuItem into a
generic shouldAppendBuiltInMenuItem helper. Rename the existing helper and
update its call site, then reuse it for the feed action so both built-in menu
items consistently skip actions already present in newWorkspaceContextMenuItems.
In `@Sources/Feed/FeedSurfaceBridge.swift`:
- Around line 60-68: Remove the unused resourceURL parameter and its discard
from isTrustedFeedURL, then update all call sites to use the simplified
signature, verifying no callers depend on passing a resource root. Preserve the
existing URL comparison based on bundledFeedToken and /feed.html.
- Around line 70-77: Replace the NSLog call in feedURL()’s registration-failure
catch block with the repository’s Logger-based logging mechanism. Preserve the
existing failure message and error details while using the appropriate logger
category or instance already established for production Swift runtime code.
In `@Sources/NewWorkspaceMenuModel.swift`:
- Around line 92-105: Consolidate the duplicated CmuxResolvedConfigMenuAction
row construction for agentChatAction and feedAction in the surrounding
new-workspace menu setup. Map over the available built-in actions, preserving
each action’s id, title, icon, iconSourcePath, tooltip, deletable status, and
isDefault comparison with newWorkspaceActionID.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 1945-1984: The synchronous directory enumeration and file
registration in registerBundledFeedAssets is being reached from the MainActor
via FeedSurfaceBridge.feedURL(). Move the scan and registration behind an
explicit background Task/queue or `@concurrent` boundary, then hop back to
MainActor only when returning the resulting feed URL; update the feedURL call
path to await or otherwise consume that asynchronous result.
In `@webviews/src/feed/App.tsx`:
- Around line 145-172: The submit button in the normalizedQuestions flow
currently enables when any answer exists, allowing incomplete payloads. Update
the enablement condition on the button using answers and normalizedQuestions so
submission is enabled only when every question has a response, while preserving
the existing perform payload and answer generation.
- Around line 22-29: Update perform to catch the thrown native error from
callFeedNative and use its specific user-facing message when available, falling
back to snapshot.copy.requestFailed otherwise. Also add and expose an in-flight
state for perform, ensuring it is set before the request and cleared after
completion so FeedActions and QuestionActions can disable permission, exit-plan,
and question controls while requests are pending.
In `@webviews/src/feed/bridge.ts`:
- Around line 57-64: Handle the promise returned by ensureSubscribed() inside
feedSnapshotStore.subscribe by attaching rejection handling, so native bridge
failures do not become unhandled global rejections. Preserve the existing
listener registration and unsubscribe behavior, and route the failure through
the established error-handling mechanism.
🪄 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: 34fc0894-5060-4927-ac88-473d7dc6aaf3
📒 Files selected for processing (33)
Resources/markdown-viewer/webviews-app/assets/feedSurface.cssResources/markdown-viewer/webviews-app/chunks/agentSessionSurface.mjsResources/markdown-viewer/webviews-app/chunks/diffSurface.mjsResources/markdown-viewer/webviews-app/chunks/feedSurface.mjsResources/markdown-viewer/webviews-app/chunks/installWebviewStyles.mjsResources/markdown-viewer/webviews-app/chunks/router.mjsResources/markdown-viewer/webviews-app/feed.htmlResources/markdown-viewer/webviews-app/main.mjsSources/AppDelegate+AgentChat.swiftSources/AppDelegate+NewWorkspaceContextMenu.swiftSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxSurfaceTabBarBuiltInAction.swiftSources/ContentView+AgentChatCommandPalette.swiftSources/ContentView.swiftSources/Feed/FeedPanelView.swiftSources/Feed/FeedPanelViewModel.swiftSources/Feed/FeedSurfaceBridge.swiftSources/NewWorkspaceMenuModel.swiftSources/Panels/BrowserPanel.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/FeedCoordinatorTests.swiftcmuxTests/NewWorkspaceMenuModelTests.swiftscripts/build-webviews-app.shwebviews/src/feed/App.tsxwebviews/src/feed/bridge.tswebviews/src/feed/styles.csswebviews/src/feed/types.tswebviews/src/main.tsxwebviews/src/router.tsxwebviews/src/surfaces/feedSurface.tsxwebviews/test/feed.test.tsx
| private func resolvedBuiltInFeedAction( | ||
| cmuxConfigStore: CmuxConfigStore | ||
| ) -> CmuxResolvedConfigAction? { | ||
| guard BrowserAvailabilitySettings.isEnabled() else { return nil } | ||
| let actionID = CmuxSurfaceTabBarBuiltInAction.feed.configID | ||
| let action = cmuxConfigStore.resolvedAction(id: actionID) ?? .builtIn(.feed) | ||
| guard action.newWorkspaceMenu != false else { return nil } | ||
| let configuredActionIDs = Set(cmuxConfigStore.newWorkspaceContextMenuItems.compactMap { item -> String? in | ||
| guard case .action(let menuAction) = item else { return nil } | ||
| return menuAction.action.id | ||
| }) | ||
| return configuredActionIDs.contains(actionID) ? nil : action | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Extract menu-item gating logic to avoid duplication.
The logic to scan cmuxConfigStore.newWorkspaceContextMenuItems and verify that the action isn't already overridden is identical to the logic in shouldAppendBuiltInNewAgentChatMenuItem. Consider renaming that helper to a generic name like shouldAppendBuiltInMenuItem and reusing it here to keep this menu-building logic DRY.
♻️ Proposed refactor
- private func resolvedBuiltInFeedAction(
- cmuxConfigStore: CmuxConfigStore
- ) -> CmuxResolvedConfigAction? {
- guard BrowserAvailabilitySettings.isEnabled() else { return nil }
- let actionID = CmuxSurfaceTabBarBuiltInAction.feed.configID
- let action = cmuxConfigStore.resolvedAction(id: actionID) ?? .builtIn(.feed)
- guard action.newWorkspaceMenu != false else { return nil }
- let configuredActionIDs = Set(cmuxConfigStore.newWorkspaceContextMenuItems.compactMap { item -> String? in
- guard case .action(let menuAction) = item else { return nil }
- return menuAction.action.id
- })
- return configuredActionIDs.contains(actionID) ? nil : action
- }
+ private func resolvedBuiltInFeedAction(
+ cmuxConfigStore: CmuxConfigStore
+ ) -> CmuxResolvedConfigAction? {
+ guard BrowserAvailabilitySettings.isEnabled() else { return nil }
+ let actionID = CmuxSurfaceTabBarBuiltInAction.feed.configID
+ let action = cmuxConfigStore.resolvedAction(id: actionID) ?? .builtIn(.feed)
+ guard shouldAppendBuiltInMenuItem(action, actionID: actionID, cmuxConfigStore: cmuxConfigStore) else { return nil }
+ return action
+ }(Note: Apply this alongside renaming the existing shouldAppendBuiltInNewAgentChatMenuItem helper on line 134 and its call site on line 110).
🤖 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/AppDelegate`+NewWorkspaceContextMenu.swift around lines 120 - 133,
Extract the shared configured-action gating from resolvedBuiltInFeedAction and
shouldAppendBuiltInNewAgentChatMenuItem into a generic
shouldAppendBuiltInMenuItem helper. Rename the existing helper and update its
call site, then reuse it for the feed action so both built-in menu items
consistently skip actions already present in newWorkspaceContextMenuItems.
| static func isTrustedFeedURL(_ url: URL?, resourceURL: URL? = Bundle.main.resourceURL) -> Bool { | ||
| _ = resourceURL | ||
| guard let url, | ||
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | ||
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | ||
| requestPath: "/feed.html" | ||
| ) else { return false } | ||
| return url == expected | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Unused resourceURL parameter on isTrustedFeedURL.
resourceURL is accepted then immediately discarded (_ = resourceURL); the trust decision only ever depends on the fixed bundledFeedToken, never on the passed-in bundle root. This is dead/no-op and could mislead a future caller into thinking the check is sandboxable by resource root.
🧹 Proposed cleanup (verify no other call sites pass `resourceURL` first)
- static func isTrustedFeedURL(_ url: URL?, resourceURL: URL? = Bundle.main.resourceURL) -> Bool {
- _ = resourceURL
- guard let url,
+ static func isTrustedFeedURL(_ url: URL?) -> Bool {
+ guard let url,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func isTrustedFeedURL(_ url: URL?, resourceURL: URL? = Bundle.main.resourceURL) -> Bool { | |
| _ = resourceURL | |
| guard let url, | |
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | |
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | |
| requestPath: "/feed.html" | |
| ) else { return false } | |
| return url == expected | |
| } | |
| static func isTrustedFeedURL(_ url: URL?) -> Bool { | |
| guard let url, | |
| let expected = CmuxDiffViewerURLSchemeHandler.diffViewerURL( | |
| token: CmuxDiffViewerURLSchemeHandler.bundledFeedToken, | |
| requestPath: "/feed.html" | |
| ) else { return false } | |
| return url == expected | |
| } |
🤖 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/Feed/FeedSurfaceBridge.swift` around lines 60 - 68, Remove the unused
resourceURL parameter and its discard from isTrustedFeedURL, then update all
call sites to use the simplified signature, verifying no callers depend on
passing a resource root. Preserve the existing URL comparison based on
bundledFeedToken and /feed.html.
| static func feedURL() -> URL? { | ||
| do { | ||
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | ||
| } catch { | ||
| NSLog("feed.surface.register.failed error=%@", String(describing: error)) | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace NSLog with Logger for the registration-failure diagnostic.
This is new production runtime code, not wrapped in #if DEBUG, so the repo's logging rule against introducing NSLog/print/etc. applies directly here.
As per coding guidelines: "Production Swift runtime code must not add print, debugPrint, dump, NSLog, ad hoc file logging, or stdout/stderr diagnostics; use Logger or the existing cmux debug log instead."
🪵 Proposed fix using Logger
+import OSLog
+
+nonisolated private let feedSurfaceBridgeLogger = Logger(subsystem: Logging.subsystem, category: "FeedSurfaceBridge")
+
`@MainActor`
final class FeedSurfaceBridge: NSObject, WKScriptMessageHandlerWithReply {
...
static func feedURL() -> URL? {
do {
return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets()
} catch {
- NSLog("feed.surface.register.failed error=%@", String(describing: error))
+ feedSurfaceBridgeLogger.error("feed.surface.register.failed error=\(error, privacy: .public)")
return nil
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func feedURL() -> URL? { | |
| do { | |
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | |
| } catch { | |
| NSLog("feed.surface.register.failed error=%@", String(describing: error)) | |
| return nil | |
| } | |
| } | |
| import OSLog | |
| nonisolated private let feedSurfaceBridgeLogger = Logger(subsystem: Logging.subsystem, category: "FeedSurfaceBridge") | |
| static func feedURL() -> URL? { | |
| do { | |
| return try CmuxDiffViewerURLSchemeHandler.shared.registerBundledFeedAssets() | |
| } catch { | |
| feedSurfaceBridgeLogger.error("feed.surface.register.failed error=\(error, privacy: .public)") | |
| return nil | |
| } | |
| } |
🤖 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/Feed/FeedSurfaceBridge.swift` around lines 70 - 77, Replace the NSLog
call in feedURL()’s registration-failure catch block with the repository’s
Logger-based logging mechanism. Preserve the existing failure message and error
details while using the appropriate logger category or instance already
established for production Swift runtime code.
Source: Coding guidelines
| if let feedAction { | ||
| createRows.append(.action( | ||
| CmuxResolvedConfigMenuAction( | ||
| id: feedAction.id, | ||
| title: feedAction.title, | ||
| icon: feedAction.icon, | ||
| iconSourcePath: feedAction.iconSourcePath, | ||
| tooltip: feedAction.tooltip, | ||
| action: feedAction | ||
| ), | ||
| deletable: deletable(feedAction), | ||
| isDefault: feedAction.id == newWorkspaceActionID | ||
| )) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consolidate built-in action row creation.
The CmuxResolvedConfigMenuAction creation block is identical to the one for agentChatAction directly above it. Consider mapping over the available built-in actions to keep the row setup DRY.
♻️ Proposed refactor
Consider replacing lines 78-105 with a compact map:
- if let agentChatAction {
- createRows.append(.action(
- CmuxResolvedConfigMenuAction(
- id: agentChatAction.id,
- title: agentChatAction.title,
- icon: agentChatAction.icon,
- iconSourcePath: agentChatAction.iconSourcePath,
- tooltip: agentChatAction.tooltip,
- action: agentChatAction
- ),
- deletable: deletable(agentChatAction),
- isDefault: agentChatAction.id == newWorkspaceActionID
- ))
- }
- if let feedAction {
- createRows.append(.action(
- CmuxResolvedConfigMenuAction(
- id: feedAction.id,
- title: feedAction.title,
- icon: feedAction.icon,
- iconSourcePath: feedAction.iconSourcePath,
- tooltip: feedAction.tooltip,
- action: feedAction
- ),
- deletable: deletable(feedAction),
- isDefault: feedAction.id == newWorkspaceActionID
- ))
- }
+ for action in [agentChatAction, feedAction].compactMap({ $0 }) {
+ createRows.append(.action(
+ CmuxResolvedConfigMenuAction(
+ id: action.id,
+ title: action.title,
+ icon: action.icon,
+ iconSourcePath: action.iconSourcePath,
+ tooltip: action.tooltip,
+ action: action
+ ),
+ deletable: deletable(action),
+ isDefault: action.id == newWorkspaceActionID
+ ))
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let feedAction { | |
| createRows.append(.action( | |
| CmuxResolvedConfigMenuAction( | |
| id: feedAction.id, | |
| title: feedAction.title, | |
| icon: feedAction.icon, | |
| iconSourcePath: feedAction.iconSourcePath, | |
| tooltip: feedAction.tooltip, | |
| action: feedAction | |
| ), | |
| deletable: deletable(feedAction), | |
| isDefault: feedAction.id == newWorkspaceActionID | |
| )) | |
| } | |
| for action in [agentChatAction, feedAction].compactMap({ $0 }) { | |
| createRows.append(.action( | |
| CmuxResolvedConfigMenuAction( | |
| id: action.id, | |
| title: action.title, | |
| icon: action.icon, | |
| iconSourcePath: action.iconSourcePath, | |
| tooltip: action.tooltip, | |
| action: action | |
| ), | |
| deletable: deletable(action), | |
| isDefault: action.id == newWorkspaceActionID | |
| )) | |
| } |
🤖 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/NewWorkspaceMenuModel.swift` around lines 92 - 105, Consolidate the
duplicated CmuxResolvedConfigMenuAction row construction for agentChatAction and
feedAction in the surrounding new-workspace menu setup. Map over the available
built-in actions, preserving each action’s id, title, icon, iconSourcePath,
tooltip, deletable status, and isDefault comparison with newWorkspaceActionID.
| func registerBundledFeedAssets(resourceURL: URL? = Bundle.main.resourceURL) throws -> URL { | ||
| guard let root = resourceURL? | ||
| .appendingPathComponent("markdown-viewer/webviews-app", isDirectory: true) | ||
| .standardizedFileURL | ||
| .resolvingSymlinksInPath(), | ||
| let enumerator = FileManager.default.enumerator( | ||
| at: root, | ||
| includingPropertiesForKeys: [.isRegularFileKey], | ||
| options: [.skipsHiddenFiles] | ||
| ) else { | ||
| throw NSError(domain: "CmuxDiffViewerURLSchemeHandler", code: 6) | ||
| } | ||
| let files = enumerator.compactMap { entry -> RegisteredFile? in | ||
| guard let fileURL = entry as? URL, | ||
| (try? fileURL.resourceValues(forKeys: [.isRegularFileKey]).isRegularFile) == true else { | ||
| return nil | ||
| } | ||
| var relativePath = String(fileURL.path.dropFirst(root.path.count)) | ||
| if relativePath.hasSuffix(".deflate") { | ||
| relativePath.removeLast(".deflate".count) | ||
| } | ||
| let mimeType: String | ||
| if relativePath.hasSuffix(".html") { | ||
| mimeType = "text/html" | ||
| } else if relativePath.hasSuffix(".mjs") || relativePath.hasSuffix(".js") { | ||
| mimeType = "text/javascript" | ||
| } else if relativePath.hasSuffix(".css") { | ||
| mimeType = "text/css" | ||
| } else { | ||
| return nil | ||
| } | ||
| return RegisteredFile(requestPath: relativePath, fileURL: fileURL, mimeType: mimeType) | ||
| } | ||
| try register(token: Self.bundledFeedToken, files: files) | ||
| guard let url = Self.diffViewerURL(token: Self.bundledFeedToken, requestPath: "/feed.html") else { | ||
| throw NSError(domain: "CmuxDiffViewerURLSchemeHandler", code: 7) | ||
| } | ||
| return url | ||
| } | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
registerBundledFeedAssets does synchronous directory-scan/file-stat I/O on the caller's actor.
This enumerates the bundle's markdown-viewer/webviews-app tree and does per-file resourceValues/fileExists/isReadableFile checks (via register) synchronously. It's invoked with no await from FeedSurfaceBridge.feedURL(), which in turn is called synchronously from a MainActor context (AppDelegate+AgentChat.performConfiguredFeedAction). This matches the project rule against putting directory-scan/file-I/O loads on the main actor without an explicit hop — consider moving the enumeration+registration to a background Task/queue and hopping back to MainActor only to hand back the resulting URL.
As per coding guidelines: "In non-test Swift, do not add or move expensive synchronous agent-history disk, JSON, transcript, trajectory, syscall, directory-scan, or unbounded-file parsing loads onto the main actor or latency-sensitive interactive paths" and "flag changed CPU-heavy, file-I/O-heavy ... helpers called from UI isolation without an explicit actor hop or @concurrent boundary."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Panels/BrowserPanel.swift` around lines 1945 - 1984, The synchronous
directory enumeration and file registration in registerBundledFeedAssets is
being reached from the MainActor via FeedSurfaceBridge.feedURL(). Move the scan
and registration behind an explicit background Task/queue or `@concurrent`
boundary, then hop back to MainActor only when returning the resulting feed URL;
update the feedURL call path to await or otherwise consume that asynchronous
result.
Source: Coding guidelines
| const perform = async (method: string, params: Record<string, unknown>) => { | ||
| setError(null); | ||
| try { | ||
| await callFeedNative(method, params); | ||
| } catch { | ||
| setError(snapshot.copy.requestFailed); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
perform() discards the specific native error and disables no in-flight state for actions.
callFeedNative throws with reply.error?.userMessage specifically so the UI can surface a targeted message, but the catch {} here ignores the caught error entirely and always falls back to the generic snapshot.copy.requestFailed. This defeats the purpose of the native userMessage field.
Separately, perform exposes no pending/in-flight flag, so callers (permission/exit-plan/question buttons in FeedActions/QuestionActions) can't disable themselves while a request is outstanding — unlike the "load more" button which uses snapshot.isLoadingOlder. Rapid double-clicks on e.g. "Allow Once" can fire duplicate feed.permission.reply calls before the snapshot updates the item's pending status.
🐛 Proposed fix to surface the specific error message
const perform = async (method: string, params: Record<string, unknown>) => {
setError(null);
try {
await callFeedNative(method, params);
- } catch {
- setError(snapshot.copy.requestFailed);
+ } catch (err) {
+ setError(err instanceof Error && err.message ? err.message : snapshot.copy.requestFailed);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const perform = async (method: string, params: Record<string, unknown>) => { | |
| setError(null); | |
| try { | |
| await callFeedNative(method, params); | |
| } catch { | |
| setError(snapshot.copy.requestFailed); | |
| } | |
| }; | |
| const perform = async (method: string, params: Record<string, unknown>) => { | |
| setError(null); | |
| try { | |
| await callFeedNative(method, params); | |
| } catch (err) { | |
| setError(err instanceof Error && err.message ? err.message : snapshot.copy.requestFailed); | |
| } | |
| }; |
🤖 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 `@webviews/src/feed/App.tsx` around lines 22 - 29, Update perform to catch the
thrown native error from callFeedNative and use its specific user-facing message
when available, falling back to snapshot.copy.requestFailed otherwise. Also add
and expose an in-flight state for perform, ensuring it is set before the request
and cleared after completion so FeedActions and QuestionActions can disable
permission, exit-plan, and question controls while requests are pending.
| const answers = normalizedQuestions.flatMap((question) => { | ||
| const custom = freeText[question.id]?.trim(); | ||
| if (custom) return [custom]; | ||
| const values = selected[question.id] ?? []; | ||
| const labels = question.options.filter((option) => values.includes(option.id)).map((option) => option.label); | ||
| return labels.length > 0 ? [labels.join(", ")] : []; | ||
| }); | ||
| return <div className="feed-question-actions"> | ||
| {normalizedQuestions.map((question) => <div className="feed-question" key={question.id}> | ||
| {question.prompt && <div className="feed-question-prompt">{question.prompt}</div>} | ||
| <div className="feed-options"> | ||
| {question.options.map((option) => ( | ||
| <button aria-pressed={(selected[question.id] ?? []).includes(option.id)} key={option.id} onClick={() => toggle(question, option.id)}> | ||
| <strong>{option.label}</strong> | ||
| {option.description && <span>{option.description}</span>} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| <input | ||
| aria-label={copy.questionPlaceholder} | ||
| onChange={(event) => setFreeText((current) => ({ ...current, [question.id]: event.target.value }))} | ||
| placeholder={copy.questionPlaceholder} | ||
| value={freeText[question.id] ?? ""} | ||
| /> | ||
| </div>)} | ||
| <button className="primary" disabled={answers.length === 0} onClick={() => perform("feed.question.reply", { itemId: item.id, selections: answers })}> | ||
| {copy.questionSubmit} | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Submit button label promises "All Answers" but enablement only requires one.
answers is computed via flatMap over normalizedQuestions, and the submit button is only disabled when answers.length === 0 — i.e. it enables once any single question has a selection or free-text answer, not once every question has one. Yet the button copy is copy.questionSubmit ("Submit All Answers"), and the payload (selections: answers) can be sent with fewer entries than questions exist for multi-question items, silently dropping unanswered questions.
Consider requiring answers.length === normalizedQuestions.length (or otherwise validating each question has a response) before enabling submission.
🤖 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 `@webviews/src/feed/App.tsx` around lines 145 - 172, The submit button in the
normalizedQuestions flow currently enables when any answer exists, allowing
incomplete payloads. Update the enablement condition on the button using answers
and normalizedQuestions so submission is enabled only when every question has a
response, while preserving the existing perform payload and answer generation.
| export const feedSnapshotStore = { | ||
| getSnapshot: () => currentSnapshot, | ||
| subscribe(listener: () => void) { | ||
| listeners.add(listener); | ||
| void ensureSubscribed(); | ||
| return () => listeners.delete(listener); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Catch the unhandled promise rejection from ensureSubscribed().
The ensureSubscribed function throws an error if the native bridge fails. Because subscribe calls it with void and no .catch(), a failure will result in an unhandled promise rejection in the global scope.
🛡️ Proposed fix to handle the rejection
export const feedSnapshotStore = {
getSnapshot: () => currentSnapshot,
subscribe(listener: () => void) {
listeners.add(listener);
- void ensureSubscribed();
+ void ensureSubscribed().catch((error) => {
+ console.error("Failed to subscribe to feed:", error);
+ });
return () => listeners.delete(listener);
},
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const feedSnapshotStore = { | |
| getSnapshot: () => currentSnapshot, | |
| subscribe(listener: () => void) { | |
| listeners.add(listener); | |
| void ensureSubscribed(); | |
| return () => listeners.delete(listener); | |
| }, | |
| }; | |
| export const feedSnapshotStore = { | |
| getSnapshot: () => currentSnapshot, | |
| subscribe(listener: () => void) { | |
| listeners.add(listener); | |
| void ensureSubscribed().catch((error) => { | |
| console.error("Failed to subscribe to feed:", error); | |
| }); | |
| return () => listeners.delete(listener); | |
| }, | |
| }; |
🤖 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 `@webviews/src/feed/bridge.ts` around lines 57 - 64, Handle the promise
returned by ensureSubscribed() inside feedSnapshotStore.subscribe by attaching
rejection handling, so native bridge failures do not become unhandled global
rejections. Preserve the existing listener registration and unsubscribe
behavior, and route the failure through the established error-handling
mechanism.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AppDelegate+AgentChat.swift (1)
131-135: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix early return when switching to an already-open Feed tab.
If the Feed workspace is already open,
executeWorkspaceCommandsuccessfully focuses it without creating a new tab. In that case,tabManager.tabswill not contain any new IDs not inbeforeIDs, causing thisguardto fail and returnfalse. This breaks the command palette flow by skippingonExecuted?()and incorrectly signaling a failure to the caller.Since
executeWorkspaceCommandreturnstrueupon success (whether it created or reused a tab), you can remove thebeforeIDsenforcement here.🐛 Proposed fix to support reusing existing tabs
- guard context.tabManager.tabs.contains(where: { !beforeIDs.contains($0.id) }) else { - return false - } onExecuted?() return true🤖 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/AppDelegate`+AgentChat.swift around lines 131 - 135, Remove the beforeIDs-based guard in executeWorkspaceCommand’s success path; rely on its successful return value so reusing an existing Feed tab still invokes onExecuted?() and returns true. Preserve the callback and success behavior for newly created tabs.
🤖 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 `@Sources/AppDelegate`+AgentChat.swift:
- Around line 131-135: Remove the beforeIDs-based guard in
executeWorkspaceCommand’s success path; rely on its successful return value so
reusing an existing Feed tab still invokes onExecuted?() and returns true.
Preserve the callback and success behavior for newly created tabs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 178003b7-1f4e-4d87-9b3b-befe497cdfb3
📒 Files selected for processing (8)
Sources/AppDelegate+AgentChat.swiftSources/CmuxConfig.swiftSources/Feed/FeedSurfaceBridge.swiftSources/Panels/BrowserPanel.swiftSources/Workspace+CustomLayout.swiftSources/WorkspaceConfigActionCapture.swiftcmuxTests/CmuxConfigTests.swiftcmuxTests/FeedCoordinatorTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Feed/FeedSurfaceBridge.swift (1)
77-84: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
registerBundledFeedAssetsis called synchronously on the MainActor, blocking it with file I/O.
feedURL()callsregisterBundledFeedAssets()synchronously. SinceFeedSurfaceBridgeis isolated to@MainActor, this blocks the main thread with a directory enumeration and per-file I/O checks.As per coding guidelines, do not add expensive synchronous directory-scan or file I/O loads onto the main actor. Move the scan and registration behind an explicit background
Taskor actor, then hop back to theMainActorwhen returning the resulting feed URL.🤖 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/Feed/FeedSurfaceBridge.swift` around lines 77 - 84, Update FeedSurfaceBridge.feedURL so registerBundledFeedAssets does not execute synchronously on the MainActor; move the directory scan and file I/O into an explicit background Task or actor, then return or deliver the resulting URL by hopping back to the MainActor while preserving the existing error logging and nil failure behavior.Source: Coding guidelines
♻️ Duplicate comments (1)
webviews/src/feed/App.tsx (1)
27-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
perform()discards the specific native error and disables no in-flight state for actions.
callFeedNativethrows withreply.error?.userMessagespecifically so the UI can surface a targeted message, but thecatch {}here ignores the caught error entirely and always falls back to the genericsnapshot.copy.requestFailed. This defeats the purpose of the nativeuserMessagefield.Separately,
performexposes no pending/in-flight flag, so callers (permission/exit-plan/question buttons inFeedActions/QuestionActions) can't disable themselves while a request is outstanding — unlike the "load more" button which usessnapshot.isLoadingOlder. Rapid double-clicks on e.g. "Allow Once" can fire duplicatefeed.permission.replycalls before the snapshot updates the item's pending status.🐛 Proposed fix to surface the specific error message
- const perform = async (method: string, params: Record<string, unknown>) => { + const perform = async (method: string, params: Record<string, unknown>) => { setError(null); try { await callFeedNative(method, params); - } catch { - setError(snapshot.copy.requestFailed); + } catch (err) { + setError(err instanceof Error && err.message ? err.message : snapshot.copy.requestFailed); } };🤖 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 `@webviews/src/feed/App.tsx` around lines 27 - 34, Update perform in App.tsx to preserve and display the specific error thrown by callFeedNative, using its native userMessage when available and snapshot.copy.requestFailed as the fallback. Also add and expose in-flight state for perform calls, then have the permission, exit-plan, and question actions in FeedActions and QuestionActions disable their controls while the corresponding request is pending, preventing duplicate submissions.
🤖 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 `@Sources/Feed/FeedSurfaceBridge.swift`:
- Around line 77-84: Update FeedSurfaceBridge.feedURL so
registerBundledFeedAssets does not execute synchronously on the MainActor; move
the directory scan and file I/O into an explicit background Task or actor, then
return or deliver the resulting URL by hopping back to the MainActor while
preserving the existing error logging and nil failure behavior.
---
Duplicate comments:
In `@webviews/src/feed/App.tsx`:
- Around line 27-34: Update perform in App.tsx to preserve and display the
specific error thrown by callFeedNative, using its native userMessage when
available and snapshot.copy.requestFailed as the fallback. Also add and expose
in-flight state for perform calls, then have the permission, exit-plan, and
question actions in FeedActions and QuestionActions disable their controls while
the corresponding request is pending, preventing duplicate submissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 83e62cfc-b7e6-4721-8fd4-233801e8a7ca
📒 Files selected for processing (12)
Resources/markdown-viewer/webviews-app/assets/feedSurface.cssResources/markdown-viewer/webviews-app/chunks/feedSurface.mjsSources/Feed/FeedSurfaceBridge.swiftSources/Panels/BrowserPanel+AutomationRecovery.swiftSources/Panels/BrowserPanel.swiftSources/Workspace.swiftcmuxTests/BrowserWebContentProcessTests.swiftcmuxTests/FeedCoordinatorTests.swiftwebviews/src/feed/App.tsxwebviews/src/feed/styles.csswebviews/src/feed/types.tswebviews/test/feed.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Feed/FeedSurfaceBridge.swift`:
- Line 199: Update FeedSurfaceBridge.snapshot() so the "items" serialization
does not map the unbounded store?.items collection on every observation update.
Apply the established pagination or hard-limit policy before itemDictionary, or
reuse a cache/diff mechanism for unchanged items, ensuring snapshot
serialization remains bounded and preserves the required feed ordering and
contents.
🪄 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: d72c35f2-bf5b-453c-9ed0-021d047a4bca
📒 Files selected for processing (7)
Resources/markdown-viewer/webviews-app/assets/feedSurface.cssResources/markdown-viewer/webviews-app/chunks/feedSurface.mjsSources/Feed/FeedSurfaceBridge.swiftwebviews/src/feed/App.tsxwebviews/src/feed/styles.csswebviews/src/feed/types.tswebviews/test/feed.test.tsx
| let background = GhosttyApp.shared.defaultBackgroundColor | ||
| let foreground = GhosttyApp.shared.defaultForegroundColor | ||
| return [ | ||
| "items": (store?.items ?? []).map(itemDictionary), |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Unbounded full-collection serialization of feed items.
snapshot() serializes the entire store?.items array via map(itemDictionary) on every update triggered by withObservationTracking. As the feed history grows (e.g., hundreds or thousands of items from loading older activity), this full-collection scan and dictionary allocation on the main actor will become a performance bottleneck, introducing UI stutter when new items arrive or states change.
As per path instructions, avoid repeated full scans and unbenchmarked algorithm choices for scalable user data like feed items and snapshot serialization.
Please consider applying a pagination/hard limit bound to the serialized array to guarantee constant-time serialization, or implement a diffing/caching mechanism that only reserializes modified items.
🤖 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/Feed/FeedSurfaceBridge.swift` at line 199, Update
FeedSurfaceBridge.snapshot() so the "items" serialization does not map the
unbounded store?.items collection on every observation update. Apply the
established pagination or hard-limit policy before itemDictionary, or reuse a
cache/diff mechanism for unchanged items, ensuring snapshot serialization
remains bounded and preserves the required feed ordering and contents.
Source: Path instructions
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 `@Resources/markdown-viewer/webviews-app/assets/feedSurface.css`:
- Line 1: Fix the source stylesheet and build lint boundary rather than editing
generated feedSurface.css directly: replace deprecated word-break: break-word
with overflow-wrap: anywhere, quote the SF Pro Text font family, and normalize
both currentColor values to the required casing. Configure Stylelint to exclude
minified generated assets or preserve lint-compatible formatting, then
regenerate the bundle.
🪄 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: 2af4e430-598e-4387-8e88-87d486e8a762
📒 Files selected for processing (2)
Resources/markdown-viewer/webviews-app/assets/feedSurface.csswebviews/src/feed/styles.css
🚧 Files skipped from review as they are similar to previous changes (1)
- webviews/src/feed/styles.css
| @@ -0,0 +1 @@ | |||
| :root{color-scheme:light dark;font:12px/1.4 -apple-system,BlinkMacSystemFont,SF Pro Text,sans-serif;--feed-background: transparent;--feed-foreground: CanvasText;--feed-border: color-mix(in srgb, var(--feed-foreground) 14%, transparent);--feed-border-strong: color-mix(in srgb, var(--feed-foreground) 38%, transparent);--feed-fill: color-mix(in srgb, var(--feed-foreground) 5%, transparent);--feed-fill-hover: color-mix(in srgb, var(--feed-foreground) 9%, transparent);--feed-muted: color-mix(in srgb, var(--feed-foreground) 56%, transparent)}*{box-sizing:border-box}html{scrollbar-gutter:stable}html,body,#root{margin:0;min-height:100%;width:100%}body{background:transparent;color:var(--feed-foreground)}button,input{font:inherit}.feed-shell{background:var(--feed-background);color:var(--feed-foreground);min-height:100vh;padding:14px clamp(12px,3vw,28px) 28px}.feed-loading{opacity:.5}.feed-header{align-items:center;border-bottom:1px solid var(--feed-border);display:flex;justify-content:space-between;margin:0 auto 8px;max-width:760px;padding-bottom:10px}.feed-header h1,.feed-card-heading h2,.feed-question-prompt{font-size:inherit;font-weight:600;margin:0}.feed-filter{border:1px solid var(--feed-border);display:flex;gap:1px;padding:2px}.feed-filter button{background:transparent;border:0;color:var(--feed-muted);cursor:pointer;padding:3px 7px}.feed-filter button:hover{background:var(--feed-fill)}.feed-filter button[aria-selected=true]{background:var(--feed-foreground);color:var(--feed-background)}.feed-list{display:grid;gap:6px;margin:0 auto;max-width:760px}.feed-card{background:color-mix(in srgb,var(--feed-background) 96%,var(--feed-foreground) 4%);border:1px solid var(--feed-border);padding:10px}.feed-card-pending{border-color:var(--feed-border-strong)}.feed-card-heading{align-items:center;display:flex;gap:10px;justify-content:space-between}.feed-card-title{align-items:center;display:flex;gap:8px;min-width:0}.feed-card-heading h2{overflow:hidden;text-overflow:ellipsis;white-space:nowrap}.feed-source{align-items:center;color:var(--feed-muted);display:inline-flex;flex:none;gap:5px}.feed-source-logo{background:currentColor;display:inline-grid;height:13px;line-height:13px;-webkit-mask-image:var(--feed-source-icon);mask-image:var(--feed-source-icon);-webkit-mask-position:center;mask-position:center;-webkit-mask-repeat:no-repeat;mask-repeat:no-repeat;-webkit-mask-size:contain;mask-size:contain;place-items:center;width:13px}.feed-source-logo[data-fallback]{background:transparent;border:1px solid currentColor;-webkit-mask-image:none;mask-image:none}.feed-card-heading time,.feed-cwd{color:var(--feed-muted);flex:none}.feed-cwd{font-family:ui-monospace,SFMono-Regular,Menlo,monospace;margin-top:6px;overflow:hidden;text-overflow:ellipsis;white-space:nowrap}.feed-body{background:var(--feed-fill);border:1px solid var(--feed-border);font:inherit;font-family:ui-monospace,SFMono-Regular,Menlo,monospace;margin:8px 0 0;max-height:240px;overflow:auto;padding:7px 8px;white-space:pre-wrap;word-break:break-word}.feed-actions{display:flex;flex-wrap:wrap;gap:5px;margin-top:8px}.feed-actions button,.feed-question-actions>button,.feed-options button,.feed-load-more{background:transparent;border:1px solid var(--feed-border-strong);color:var(--feed-foreground);cursor:pointer;padding:4px 7px}.feed-actions button:hover,.feed-question-actions>button:hover,.feed-options button:hover,.feed-load-more:hover{background:var(--feed-fill-hover)}.feed-actions .primary,.feed-question-actions .primary{background:var(--feed-foreground);border-color:var(--feed-foreground);color:var(--feed-background)}.feed-actions .primary:hover,.feed-question-actions .primary:hover{opacity:.82}.feed-actions .danger{border-style:dashed}.feed-question-actions{display:grid;gap:7px;margin-top:8px}.feed-question{display:grid;gap:6px}.feed-question input{background:var(--feed-fill);border:1px solid var(--feed-border-strong);color:var(--feed-foreground);padding:5px 7px;width:100%}.feed-options{display:grid;gap:4px}.feed-options button{display:grid;gap:1px;text-align:left}.feed-options button[aria-pressed=true]{background:var(--feed-foreground);color:var(--feed-background)}.feed-options span{color:var(--feed-muted)}.feed-options button[aria-pressed=true] span{color:color-mix(in srgb,var(--feed-background) 70%,transparent)}.feed-empty{color:var(--feed-muted);padding:44px 12px;text-align:center}.feed-error{border:1px solid var(--feed-border-strong);margin:0 auto 6px;max-width:760px;padding:6px 8px}.feed-load-more{justify-self:center;margin-top:2px}button:disabled{cursor:default;opacity:.4}button:focus-visible,.feed-question input:focus-visible{outline:1px solid var(--feed-foreground);outline-offset:2px}@media(max-width:540px){.feed-header{align-items:stretch;flex-direction:column;gap:8px}.feed-filter{align-self:flex-start}.feed-card-title{align-items:flex-start;flex-direction:column;gap:3px}} | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve Stylelint failures in the generated stylesheet.
Line 1 contains four reported violations: deprecated word-break: break-word, unquoted SF Pro Text, and two currentColor casing errors. Since linting is part of the PR verification, fix the source stylesheet/build lint boundary—not this generated bundle directly—and regenerate it. Replace word-break: break-word with overflow-wrap: anywhere, and exclude minified generated assets from Stylelint or ensure the bundler preserves the required formatting.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 1-1: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
[error] 1-1: Expected quotes around "SF Pro Text" (font-family-name-quotes)
(font-family-name-quotes)
[error] 1-1: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
[error] 1-1: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 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 `@Resources/markdown-viewer/webviews-app/assets/feedSurface.css` at line 1, Fix
the source stylesheet and build lint boundary rather than editing generated
feedSurface.css directly: replace deprecated word-break: break-word with
overflow-wrap: anywhere, quote the SF Pro Text font family, and normalize both
currentColor values to the required casing. Configure Stylelint to exclude
minified generated assets or preserve lint-compatible formatting, then
regenerate the bundle.
Source: Linters/SAST tools
# Conflicts: # Resources/markdown-viewer/webviews-app/chunks/diffSurface.mjs # Sources/Panels/BrowserPanel.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
# Conflicts: # Sources/CoalesceLatestPublisher.swift
# Conflicts: # Sources/Panels/BrowserPanel+AutomationRecovery.swift # Sources/Panels/BrowserPanel.swift # Sources/Workspace.swift
lawrencecchen
added
the
stale-revisit
2 participants
Summary
Verification
fdrxand verified the React DOM and custom-scheme URLbun run typecheckbun test test/feed.test.tsxbun run lint:ci./scripts/build-webviews-app.sh --check./scripts/check-pbxproj.shNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a React Feed accessible from the workspace menu and command palette. It opens in a trusted, chrome-less tab, matches the terminal theme, and improves keyboard focus and session restore.
New Features
cmux.feedto the workspace dropdown and command palette; opens Feed in a chrome-less trusted tab viacmux-diff-viewer./feedwith bundledfeed.html; introducesFeedSurfaceBridgewith typed native calls scoped to trusted Feed surfaces only.omnibarVisibleis captured and restored in layouts.AsyncPublisherdemand.Migration
cmux.feedto wire custom menus or shortcuts.Written for commit 8be94a2. Summary will update on new commits.
Summary by CodeRabbit