Repository navigation
Organize extension SDK boundaries - #5085
Merged
Merged
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:
📝 WalkthroughWalkthroughAdds CmuxSidebarProviderKit, renames and SPI-gates many Cmux extension types to a provider model, refactors host transport to async/throws, migrates app UI, examples, and tests to provider contracts, and updates packages, Xcode projects, scripts, and localizations. ChangesProvider-kit migration and host wiring
Sequence Diagram(s)sequenceDiagram
participant AppUI as App UI
participant Provider as CmuxSidebarProvider
participant Host as CmuxSidebarHost
AppUI->>Provider: render(snapshot: CmuxSidebarProviderSnapshot)
Provider-->>AppUI: CmuxSidebarProviderRenderModel
AppUI->>Host: send action (select/create/open) as CmuxSidebarAction
Host-->>AppUI: CmuxSidebarActionResult or throws CmuxSidebarActionError
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
Greptile SummaryThis PR reorganizes the extension SDK into two clear layers —
Confidence Score: 4/5Safe to merge after adding the 18 missing locale translations for the two new .createWorkspaceWithPath permission strings. The SDK restructuring, async host API, cancellation bridge, and new package boundaries are all well-implemented. The one issue that needs fixing before ship is the localization gap: the two new permission-UI strings shown to users in the access-review sheet and permissions popover only carry English and Japanese translations, leaving users in all other supported locales with untranslated key identifiers on screen. Resources/Localizable.xcstrings — the two new createWorkspaceWithPath entries need ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant translations. Important Files Changed
Sequence DiagramsequenceDiagram
participant Ext as Extension (@main)
participant Scene as CmuxSidebarExtensionScene
participant Runtime as CmuxSidebarExtensionRuntime
participant Conn as CMUXSidebarExtensionConnection
participant Host as CmuxSidebarHost (async API)
participant XPC as XPC / cmux host
Ext->>Scene: body evaluated
Scene->>Runtime: "init(sidebarExtension:) @MainActor"
Runtime->>Conn: init(manifest:onSnapshot:onStatus:)
XPC-->>Conn: accept(NSXPCConnection)
Conn-->>Ext: connectionStatusDidChange(.connected)
XPC-->>Conn: sidebarSnapshotDidChange(payload)
Conn->>Runtime: "onSnapshot(snapshot) @MainActor"
Runtime->>Host: CmuxSidebarHost(performCancellableAction:)
Runtime->>Ext: update(context: CmuxSidebarContext)
Ext->>Host: selectWorkspace(id) async throws
Host->>Conn: perform(action, reply:)
Conn->>XPC: performSidebarAction(payload, reply:)
XPC-->>Conn: reply(resultPayload, nil)
Conn->>Host: reply(CmuxSidebarActionResult)
Host-->>Ext: returns / throws CmuxSidebarActionError
Reviews (5): Last reviewed commit: "Document sidebar provider kit models" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
57
to
69
| return CMUXSidebarSnapshot( | ||
| apiVersion: apiVersion, | ||
| sequence: sequence, | ||
| windowID: windowID, | ||
| selectedWorkspaceID: selectedWorkspaceID, | ||
| windowID: scopeSet.contains(.workspaceMetadata) ? windowID : nil, | ||
| selectedWorkspaceID: scopeSet.contains(.workspaceMetadata) ? selectedWorkspaceID : nil, | ||
| grantedReadScopes: scopeSet, | ||
| grantedActionScopes: actionScopeSet, | ||
| workspaces: workspaces.map { workspace in | ||
| scopeSet.contains(.workspaceMetadata) | ||
| ? workspace.filtered(for: scopeSet) | ||
| : CMUXSidebarWorkspace(id: workspace.id, title: workspace.title) | ||
| : CMUXSidebarWorkspace(id: workspace.id, title: "") | ||
| } | ||
| ) |
Contributor
There was a problem hiding this comment.
workspaceList scope now silently strips workspace titles — breaking change for existing extensions
Before this change, an extension granted only .workspaceList received workspace.title from the snapshot. After it, the title is forcibly cleared to "". Any existing third-party or in-house extension that renders or logs workspace names using only the workspaceList scope will silently show empty strings without any compile-time or runtime error. The README only subtly signals this via "workspace identities and ordering only", which is easy to miss. It would help to add a runtime assertion or debug-only warning when an extension requests workspaceList but not workspaceMetadata, or at minimum document the behavioral break in the migration notes / CHANGELOG before shipping.
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!
Comment on lines
4
to
+6
| @main | ||
| final class SampleSidebarExtension: CmuxSidebarExtension { | ||
| @MainActor | ||
| final class SampleSidebarExtension: @MainActor CmuxSidebarExtension { |
Contributor
There was a problem hiding this comment.
@MainActor on the protocol conformance — the class declaration already marks every member @MainActor, so the attribute on the inheritance entry is a no-op and could confuse readers or generate a future compiler diagnostic about duplicate actor annotations.
Suggested change
| @main | |
| final class SampleSidebarExtension: CmuxSidebarExtension { | |
| @MainActor | |
| final class SampleSidebarExtension: @MainActor CmuxSidebarExtension { | |
| @main | |
| @MainActor | |
| final class SampleSidebarExtension: CmuxSidebarExtension { |
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!
Comment on lines
17
to
18
| @_spi(CmuxLegacyHostActions) | ||
| public let cmux: CmuxHost |
Contributor
There was a problem hiding this comment.
init eagerly allocates a CmuxHost fallback (inside the SPI init via cmux ?? CmuxHost { ... }) even though callers without @_spi(CmuxLegacyHostActions) can never access context.cmux. The allocation is cheap (it's a struct + closure), but a lazy backing store or an internal Optional<CmuxHost> would make the "this field is inactive in the public API" intent clearer and avoid the unnecessary closure capture.
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: 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/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swift`:
- Around line 20-21: The `@_spi`(CmuxHostTransport) on the initializer is
redundant because the actor CMUXSidebarExtensionSession is already annotated
with that SPI; remove the annotation from the init declaration (public
init(...)) so the initializer inherits the actor's SPI implicitly, leaving the
actor-level `@_spi`(CmuxHostTransport) only; if you prefer keeping it for explicit
documentation you can alternatively leave a brief comment, but do not duplicate
the attribute on CMUXSidebarExtensionSession.init.
🪄 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: 786bca21-cfbd-4c7d-aabf-0c980d220836
📒 Files selected for processing (44)
Examples/CmuxExtensionSidebarExamples/Package.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/AttentionQueueSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/BrowserStackSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/DevServerSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/LastPromptSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/ProjectWorktreeSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SidebarExamples.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SuperCompactSidebar.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/BrowserStackSidebarTests.swiftExamples/SampleSidebarExtensionApp/README.mdExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Extension/SampleSidebarExtension.swiftExamples/StubAgentSidebarExtension/Package.swiftExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/Resources/Localizable.xcstringsExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swiftExamples/TabsVisibleSidebar/TabsVisibleSidebar.xcodeproj/project.pbxprojPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Browser/CMUXSidebarExtensionBrowserPresenter.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXInstalledSidebarExtension.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXSidebarExtensionDiscovery.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXExtensionClientError.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRecord.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRegistry.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swiftPackages/CMUXExtensionClient/Tests/CMUXExtensionClientTests/CMUXExtensionClientTests.swiftPackages/CmuxExtensionKit/README.mdPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxHost.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarAction.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarActionCancellation.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarContext.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarHost.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/Package.swiftPackages/CmuxSidebarProviderKit/README.mdPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxExtensionCompatibility.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/HostCompatibility/CmuxExtensionLocalizedText.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/HostCompatibility/CmuxExtensionSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/HostCompatibility/CmuxExtensionSidebarProviderDescriptor.swiftSources/ContentView.swiftSources/ExtensionSidebarWorkspaceRowView.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- Examples/TabsVisibleSidebar/TabsVisibleSidebar.xcodeproj/project.pbxproj
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 44 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift (1)
3-23: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMissing Swift-DocC documentation for public types.
Both
CmuxSidebarActionResultandCmuxSidebarActionErrorare public API but lack documentation. Add triple-slash comments describing their purpose and cases.📝 Proposed documentation
`@_spi`(CmuxHostTransport) +/// Result of a sidebar action request sent to the CMUX host. public struct CmuxSidebarActionResult: Codable, Equatable, Sendable { + /// Whether the host accepted and processed the action. public var accepted: Bool + /// Optional message describing the result or rejection reason. public var message: String? ... } +/// Errors that can occur when performing a sidebar action. public enum CmuxSidebarActionError: Error, Equatable, Sendable { + /// The host rejected the action with the given reason. case rejected(String) + /// The action was cancelled before completion. case cancelled }🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift` around lines 3 - 23, Add Swift-DocC triple‑slash documentation comments to the public types: describe CmuxSidebarActionResult (its role as the result of a sidebar action, meanings of the accepted Bool, optional message, the static accepted constant, and the rejected(_:) constructor) and CmuxSidebarActionError (explain each error case: .rejected(String) with the reason and .cancelled for user/cancellation). Place the comments immediately above the declarations for CmuxSidebarActionResult and CmuxSidebarActionError so they appear in generated docs and Xcode Quick Help.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift (1)
3-5: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMissing safety comment for
@unchecked Sendable.The class is marked
@unchecked Sendablebut lacks the required safety argument comment. Document why this is safe (e.g., the stored properties and their thread-safety guarantees).📝 Proposed safety comment
+/// Runtime that bridges a `CmuxSidebarExtension` to the XPC transport. +/// +/// - Note: `@unchecked Sendable`: `sidebarExtension` is `@MainActor`-isolated and accessed +/// only via `@MainActor` closures; `connection` is itself `Sendable`. final class CmuxSidebarExtensionRuntime<Extension: CmuxSidebarExtension>: `@unchecked` Sendable {As per coding guidelines: "
@uncheckedSendable and nonisolated(unsafe) require comment on declaration explaining safety argument."🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift` around lines 3 - 5, The declaration of CmuxSidebarExtensionRuntime is marked `@unchecked` Sendable but lacks the required safety justification comment; add a brief comment directly above the class declaration explaining why it's safe to bypass automatic Sendable checking (e.g., that the stored properties sidebarExtension and connection are themselves thread-safe or only accessed on a single serial executor, that no mutable shared state is exposed, and any required synchronization is handled elsewhere), and reference the relevant symbols (sidebarExtension, connection, and the Extension generic) so reviewers can verify the safety argument.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swift (1)
3-14: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding DocC comments for SPI-gated public symbols.
While this type is gated under
@_spi(CmuxHostTransport), adding brief documentation (e.g., purpose ofsnapshotvsdispatchclosures) would help host-side consumers understand the contract.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swift` around lines 3 - 14, The SPI-exposed type CmuxSidebarHostClient lacks DocC comments explaining its role and the semantics of its closures; add brief documentation comments to the struct and its public members (CmuxSidebarHostClient, snapshot, dispatch) describing the host-side purpose of the client, what snapshot() returns and when it should be called, and what dispatch(action) does and what CmuxSidebarActionResult represents so consumers understand the contract and expected behavior.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swift (2)
27-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing safety comment for
@unchecked Sendable.Per coding guidelines,
@unchecked Sendablerequires a comment explaining the safety argument. Add a comment describing why this type is safe to send across isolation boundaries (e.g., "Runtime handles its own isolation via XPC connection serialization").As per coding guidelines: "
@uncheckedSendable and nonisolated(unsafe) require comment on declaration explaining safety argument; use 'Wraps X; every mutation happens on Y' or 'UserDefaults is thread-safe' pattern."🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swift` at line 27, Add a concise safety comment to the SidebarRuntimeBox declaration explaining why `@unchecked` Sendable is safe: mention the generic type SidebarRuntimeBox<Extension: CmuxSidebarExtension>, that it conforms to AnySidebarRuntimeBox, and state the specific isolation guarantee (e.g., "Wraps Runtime; all mutation happens on the XPC/actor thread and access is serialized by the runtime's XPC connection") so reviewers can verify thread-safety; place this comment immediately before the declaration of SidebarRuntimeBox to satisfy the guideline requiring a safety rationale for `@unchecked` Sendable.
39-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing safety comment for
@unchecked Sendable.Same as above—add a brief comment explaining why
AnySidebarRuntimeBoxis safe to share across isolation domains.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swift` at line 39, Add a brief safety rationale comment explaining why AnySidebarRuntimeBox is annotated with `@unchecked` Sendable: state which internal stored properties or captured references are immutable or themselves Sendable (e.g., immutable closures, value types, or thread-safe objects) and note that no non-sendable mutable state is accessed across concurrency domains; place this comment immediately above the declaration of the private class AnySidebarRuntimeBox to document why it's safe to share across isolation boundaries.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swift (1)
3-11: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding DocC comments for public API.
This is a new public type in
Packages/. A brief triple-slash comment explaining the intended usage (e.g., "Represents a localizable string with its key and fallback value for sidebar provider rendering") would help extension authors.As per coding guidelines: "Every
publicsymbol in new Swift packages under Packages/ must be documented with Swift-DocC triple-slash comment at 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swift` around lines 3 - 11, Add Swift-DocC triple-slash comments to the public type CmuxSidebarProviderLocalizedText and its public members: document the struct with a brief summary like "Represents a localizable string with its key and fallback value for sidebar provider rendering", then add brief comments for the properties key and defaultValue and the public init describing their purpose and parameters; ensure every public symbol (CmuxSidebarProviderLocalizedText, key, defaultValue, init) has a /// comment immediately above it to satisfy the Packages/ documentation guideline.
🤖 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
`@Examples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swift`:
- Around line 33-35: The click handlers currently swallow failures by using try?
when calling selectWorkspace(workspace.id) and createWorkspace(...); change them
to capture errors and surface user feedback: add an observable errorText (or
reuse existing state) and update the Task blocks to await the call inside a
do/catch that sets errorText on failure and handles
CmuxSidebarActionError.rejected specifically (or use the existing apply helper
pattern shown in Sample/TabsVisible) so users see an error message instead of
silent failure.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift`:
- Around line 3-8: The public API CmuxExtensionManifest and its public
properties (id, displayName, minimumAPIVersion, readScopes, actionScopes) lack
Swift-DocC documentation; add triple-slash (///) doc comments above the struct
declaration and above each public property describing their purpose,
units/format (e.g., id format), semantics (what displayName represents),
expected values/constraints for minimumAPIVersion, and what
readScopes/actionScopes control so the public symbols are documented per package
guidelines.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtension.swift`:
- Around line 5-9: Add Swift-DocC documentation to the public enum
CmuxSidebarConnectionStatus explaining its overall purpose (representing the
sidebar connection state) and document each case (connected, waitingForHost,
error(String)) including what the associated String in error represents and when
waitingForHost applies; place /// comments immediately above the enum and each
case so Xcode/DocC will surface the descriptions.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift`:
- Line 8: The declaration of CMUXSidebarExtensionConnection uses `@unchecked`
Sendable but lacks the required safety comment; add a concise comment on the
class explaining why it's safe to be `@unchecked` Sendable (e.g., state is
protected by an NSLock instance and all non-sendable mutation happens under that
lock, or only immutable/sendable data is shared), and reference the NSLock used
for synchronization (and any other thread-affine or non-sendable members) so
reviewers can verify the safety argument; keep the comment adjacent to the class
declaration containing the `@unchecked` Sendable attribute.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarHost.swift`:
- Around line 140-142: The code uses a fragile literal "Extension action was
cancelled" to detect cancellation (checked against message and thrown as
CmuxSidebarActionError.cancelled); extract that literal into a single shared
constant (e.g., cancellationMessage) in CmuxSidebarHost and replace every
literal comparison/usage (the if message == ... check and the other occurrence
that constructs/returns that text) to use the constant instead; alternatively,
replace the string-based signaling with a structured result or error enum
consistently, but at minimum introduce and reference cancellationMessage
wherever the cancellation text is produced or checked so they cannot drift
independently.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProvider.swift`:
- Around line 3-5: Add Swift-DocC triple-slash documentation to the public
protocol CmuxSidebarProvider and its public member descriptor: write a brief ///
summary above the protocol declaration describing its role and contract
(including that it is Sendable) and add a /// comment for the descriptor
property explaining what the CmuxSidebarProviderDescriptor represents and how
callers should use it; ensure the comments follow the project's DocC style and
appear directly above the protocol and the var descriptor declaration.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift`:
- Around line 3-33: Add Swift-DocC triple-slash documentation for the public
struct and all its public members: document CmuxSidebarProviderDescriptor itself
and its static defaultWorkspacesID and defaultWorkspaces, the properties id,
title, subtitle, systemImageName, isHostProvided, and the public init(...)
initializer; include a short description for each symbol, document parameter
names and meanings for the initializer, and ensure comments follow the
triple-slash format (///) and concise DocC style so the package complies with
the public-symbol documentation guideline.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderModels.swift`:
- Around line 3-6: Public types and protocols in CmuxSidebarProviderModels.swift
lack Swift-DocC triple-slash documentation; add concise one-line DocC summaries
for each public symbol listed (e.g., CmuxSidebarProviderPresentation,
CmuxSidebarProviderWorkspacePopoverTab, CmuxSidebarProviderRelativeDateStyle,
CmuxSidebarProviderIconShape, CmuxSidebarProviderText,
CmuxContextualSidebarProvider, CmuxMutableSidebarProvider,
CmuxSidebarProviderGitBranch) and any other public enums/structs/protocols noted
in the review ranges so they comply with the package guideline; place ///
comments immediately above each declaration with a short descriptive sentence,
and include brief documentation on public protocol methods where applicable.
In `@scripts/write-sidebar-extension-point.sh`:
- Around line 31-37: The generated appextensionpoint plist currently emits only
EXExtensionPointIsPublic and EXPresentsUserInterface at top-level, but
ExtensionKit requires the extension-point identifier to be the top-level key
(e.g., use the POINT_ID variable as the top-level key with the configuration
dict nested under it); update scripts/write-sidebar-extension-point.sh so the
plist root is a dict with a single key of the extension point identifier whose
value is the dict containing EXExtensionPointIsPublic and
EXPresentsUserInterface (and if applicable reintroduce the _EXScopeRestriction
key into that nested dict rather than removing it, or document/justify its
removal to avoid widening attachment scope). Ensure you reference the POINT_ID
variable (or constant used in the script) when emitting the top-level key and
include _EXScopeRestriction back under that nested dict unless you have an
explicit reason to drop it.
---
Outside diff comments:
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`:
- Around line 3-23: Add Swift-DocC triple‑slash documentation comments to the
public types: describe CmuxSidebarActionResult (its role as the result of a
sidebar action, meanings of the accepted Bool, optional message, the static
accepted constant, and the rejected(_:) constructor) and CmuxSidebarActionError
(explain each error case: .rejected(String) with the reason and .cancelled for
user/cancellation). Place the comments immediately above the declarations for
CmuxSidebarActionResult and CmuxSidebarActionError so they appear in generated
docs and Xcode Quick Help.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift`:
- Around line 3-5: The declaration of CmuxSidebarExtensionRuntime is marked
`@unchecked` Sendable but lacks the required safety justification comment; add a
brief comment directly above the class declaration explaining why it's safe to
bypass automatic Sendable checking (e.g., that the stored properties
sidebarExtension and connection are themselves thread-safe or only accessed on a
single serial executor, that no mutable shared state is exposed, and any
required synchronization is handled elsewhere), and reference the relevant
symbols (sidebarExtension, connection, and the Extension generic) so reviewers
can verify the safety argument.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swift`:
- Line 27: Add a concise safety comment to the SidebarRuntimeBox declaration
explaining why `@unchecked` Sendable is safe: mention the generic type
SidebarRuntimeBox<Extension: CmuxSidebarExtension>, that it conforms to
AnySidebarRuntimeBox, and state the specific isolation guarantee (e.g., "Wraps
Runtime; all mutation happens on the XPC/actor thread and access is serialized
by the runtime's XPC connection") so reviewers can verify thread-safety; place
this comment immediately before the declaration of SidebarRuntimeBox to satisfy
the guideline requiring a safety rationale for `@unchecked` Sendable.
- Line 39: Add a brief safety rationale comment explaining why
AnySidebarRuntimeBox is annotated with `@unchecked` Sendable: state which internal
stored properties or captured references are immutable or themselves Sendable
(e.g., immutable closures, value types, or thread-safe objects) and note that no
non-sendable mutable state is accessed across concurrency domains; place this
comment immediately above the declaration of the private class
AnySidebarRuntimeBox to document why it's safe to share across isolation
boundaries.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swift`:
- Around line 3-14: The SPI-exposed type CmuxSidebarHostClient lacks DocC
comments explaining its role and the semantics of its closures; add brief
documentation comments to the struct and its public members
(CmuxSidebarHostClient, snapshot, dispatch) describing the host-side purpose of
the client, what snapshot() returns and when it should be called, and what
dispatch(action) does and what CmuxSidebarActionResult represents so consumers
understand the contract and expected behavior.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swift`:
- Around line 3-11: Add Swift-DocC triple-slash comments to the public type
CmuxSidebarProviderLocalizedText and its public members: document the struct
with a brief summary like "Represents a localizable string with its key and
fallback value for sidebar provider rendering", then add brief comments for the
properties key and defaultValue and the public init describing their purpose and
parameters; ensure every public symbol (CmuxSidebarProviderLocalizedText, key,
defaultValue, init) has a /// comment immediately above it to satisfy the
Packages/ documentation guideline.
🪄 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: e016a4b9-a03d-40d4-8604-ed384dea5ca4
📒 Files selected for processing (74)
Examples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/AttentionQueueSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/BrowserStackSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/DevServerSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/LastPromptSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/ProjectWorktreeSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SidebarExamples.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SuperCompactSidebar.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/BrowserStackSidebarTests.swiftExamples/SampleSidebarExtensionApp/README.mdExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Extension/SampleSidebarExtension.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Localizable.xcstringsExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Model/SidebarInsightModel.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/UI/SampleSidebarView.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtensionApp/Localizable.xcstringsExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swiftExamples/TabsVisibleSidebar/TabsVisibleSidebar.xcodeproj/project.pbxprojExamples/TabsVisibleSidebar/TabsVisibleSidebar/Localizable.xcstringsExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/Localizable.xcstringsExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarExtension.swiftExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarView.swiftPackages/CMUXExtensionClient/README.mdPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXInstalledSidebarExtension.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXSidebarExtensionDiscovery.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXExtensionClientError.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRecord.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRegistry.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swiftPackages/CMUXExtensionClient/Tests/CMUXExtensionClientTests/CMUXExtensionClientTests.swiftPackages/CMUXExtensionHostSupport/Package.swiftPackages/CMUXExtensionHostSupport/README.mdPackages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/Browser/CMUXSidebarExtensionBrowserPresenter.swiftPackages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/Hosting/CMUXSidebarExtensionHostView.swiftPackages/CmuxExtensionKit/README.mdPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxHost.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxUIExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionKind.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionScope.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarAction.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSurface.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarWorkspace.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarXPC.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarContext.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarHost.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidationError.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidator.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/README.mdPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderModels.swiftResources/Localizable.xcstringsResources/com.manaflow.cmux.sidebar.appextensionpointSources/CMUXInstalledExtensionSidebarHostView.swiftSources/CMUXSidebarExtensionBrowserPanel.swiftSources/ContentView.swiftSources/ExtensionSidebarWorkspaceRowView.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatascripts/reload.shscripts/write-sidebar-extension-point.sh
💤 Files with no reviewable changes (14)
- Packages/CMUXExtensionClient/README.md
- Packages/CMUXExtensionClient/Tests/CMUXExtensionClientTests/CMUXExtensionClientTests.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxHost.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionKind.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxUIExtension.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXInstalledSidebarExtension.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXSidebarExtensionDiscovery.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxExtension.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRegistry.swift
- Resources/com.manaflow.cmux.sidebar.appextensionpoint
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXExtensionClientError.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRecord.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swift
- Packages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/Browser/CMUXSidebarExtensionBrowserPresenter.swift
Comment on lines
3
to
6
| public enum CmuxSidebarProviderPresentation: String, Codable, Equatable, Sendable { | ||
| case tree | ||
| case browserStack = "browser-stack" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Missing DocC documentation on public types in models file.
Multiple public enums, structs, and protocols in this file lack Swift-DocC documentation. Key types needing documentation include:
CmuxSidebarProviderPresentationCmuxSidebarProviderWorkspacePopoverTabCmuxSidebarProviderRelativeDateStyleCmuxSidebarProviderIconShapeCmuxSidebarProviderTextCmuxContextualSidebarProvider/CmuxMutableSidebarProviderCmuxSidebarProviderGitBranch
Consider adding at minimum a one-line summary for each public type and protocol method.
As per coding guidelines: "Every public symbol in new Swift packages under Packages/ must be documented with Swift-DocC triple-slash comment at time of writing."
Also applies to: 39-43, 71-73, 75-78, 102-115, 229-238, 280-288
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderModels.swift`
around lines 3 - 6, Public types and protocols in
CmuxSidebarProviderModels.swift lack Swift-DocC triple-slash documentation; add
concise one-line DocC summaries for each public symbol listed (e.g.,
CmuxSidebarProviderPresentation, CmuxSidebarProviderWorkspacePopoverTab,
CmuxSidebarProviderRelativeDateStyle, CmuxSidebarProviderIconShape,
CmuxSidebarProviderText, CmuxContextualSidebarProvider,
CmuxMutableSidebarProvider, CmuxSidebarProviderGitBranch) and any other public
enums/structs/protocols noted in the review ranges so they comply with the
package guideline; place /// comments immediately above each declaration with a
short descriptive sentence, and include brief documentation on public protocol
methods where applicable.
There was a problem hiding this comment.
8 issues found across 76 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift">
<violation number="1" location="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift:36">
P2: Decoder is not backward-compatible with previous manifest key `requestedScopes`, so older manifests now fail to load.</violation>
</file>
<file name="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift">
<violation number="1" location="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift:20">
P1: Info.plist key string mismatch: the renamed `identifierInfoPlistKey` (`"CmuxSidebarExtensionPointIdentifier"`) does not match the key still declared in `./Resources/Info.plist` (`CMUXSidebarExtensionPointIdentifier`). This causes `Bundle.object(forInfoDictionaryKey:)` to always return nil, silently falling back to `baseIdentifier` and bypassing any build-time override.</violation>
</file>
<file name="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swift">
<violation number="1" location="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swift:3">
P3: File name `CMUXExtensionAPIVersion.swift` no longer matches the renamed struct `CmuxExtensionAPIVersion`. Rename the file to `CmuxExtensionAPIVersion.swift` to follow Swift naming conventions where the file name matches the primary type name.</violation>
</file>
<file name="Examples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swift">
<violation number="1" location="Examples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swift:28">
P3: Map known host error strings to localized text instead of displaying raw transport messages.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| /// (via Info.plist variable substitution), so the resolved id lives only in the built | ||
| /// bundle, never in tracked source. Absent or empty means "use ``baseIdentifier``". | ||
| public static let identifierInfoPlistKey = "CMUXSidebarExtensionPointIdentifier" | ||
| public static let identifierInfoPlistKey = "CmuxSidebarExtensionPointIdentifier" |
There was a problem hiding this comment.
P1: Info.plist key string mismatch: the renamed identifierInfoPlistKey ("CmuxSidebarExtensionPointIdentifier") does not match the key still declared in ./Resources/Info.plist (CMUXSidebarExtensionPointIdentifier). This causes Bundle.object(forInfoDictionaryKey:) to always return nil, silently falling back to baseIdentifier and bypassing any build-time override.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift, line 20:
<comment>Info.plist key string mismatch: the renamed `identifierInfoPlistKey` (`"CmuxSidebarExtensionPointIdentifier"`) does not match the key still declared in `./Resources/Info.plist` (`CMUXSidebarExtensionPointIdentifier`). This causes `Bundle.object(forInfoDictionaryKey:)` to always return nil, silently falling back to `baseIdentifier` and bypassing any build-time override.</comment>
<file context>
@@ -16,7 +17,7 @@ public enum CMUXSidebarExtensionPoint {
/// (via Info.plist variable substitution), so the resolved id lives only in the built
/// bundle, never in tracked source. Absent or empty means "use ``baseIdentifier``".
- public static let identifierInfoPlistKey = "CMUXSidebarExtensionPointIdentifier"
+ public static let identifierInfoPlistKey = "CmuxSidebarExtensionPointIdentifier"
/// Resolves the extension point identifier for a bundle.
</file context>
Suggested change
| public static let identifierInfoPlistKey = "CmuxSidebarExtensionPointIdentifier" | |
| public static let identifierInfoPlistKey = "CMUXSidebarExtensionPointIdentifier" |
| [CMUXExtensionActionScope].self, | ||
| forKey: .requestedActionScopes | ||
| minimumAPIVersion = try container.decodeIfPresent(CmuxExtensionAPIVersion.self, forKey: .minimumAPIVersion) ?? .sidebarV1 | ||
| readScopes = try container.decode([CmuxExtensionScope].self, forKey: .readScopes) |
There was a problem hiding this comment.
P2: Decoder is not backward-compatible with previous manifest key requestedScopes, so older manifests now fail to load.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift, line 36:
<comment>Decoder is not backward-compatible with previous manifest key `requestedScopes`, so older manifests now fail to load.</comment>
<file context>
@@ -1,65 +1,42 @@
- [CMUXExtensionActionScope].self,
- forKey: .requestedActionScopes
+ minimumAPIVersion = try container.decodeIfPresent(CmuxExtensionAPIVersion.self, forKey: .minimumAPIVersion) ?? .sidebarV1
+ readScopes = try container.decode([CmuxExtensionScope].self, forKey: .readScopes)
+ actionScopes = try container.decodeIfPresent(
+ [CmuxExtensionActionScope].self,
</file context>
Collaborator
Author
There was a problem hiding this comment.
Resolved by bumping the manifest/snapshot contract to CmuxExtensionAPIVersion.sidebarV2 and removing sidebarV1 from the public SDK default path. We are intentionally not adding requestedScopes/requestedActionScopes aliases because extensions have not shipped publicly yet.
— Claude Code
| import Foundation | ||
|
|
||
| public struct CMUXExtensionAPIVersion: Codable, Comparable, Equatable, Sendable { | ||
| public struct CmuxExtensionAPIVersion: Codable, Comparable, Equatable, Sendable { |
There was a problem hiding this comment.
P3: File name CMUXExtensionAPIVersion.swift no longer matches the renamed struct CmuxExtensionAPIVersion. Rename the file to CmuxExtensionAPIVersion.swift to follow Swift naming conventions where the file name matches the primary type name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swift, line 3:
<comment>File name `CMUXExtensionAPIVersion.swift` no longer matches the renamed struct `CmuxExtensionAPIVersion`. Rename the file to `CmuxExtensionAPIVersion.swift` to follow Swift naming conventions where the file name matches the primary type name.</comment>
<file context>
@@ -1,6 +1,6 @@
import Foundation
-public struct CMUXExtensionAPIVersion: Codable, Comparable, Equatable, Sendable {
+public struct CmuxExtensionAPIVersion: Codable, Comparable, Equatable, Sendable {
public var major: Int
public var minor: Int
</file context>
| case .waitingForHost: | ||
| errorText = String(localized: "sampleSidebar.waitingForHost", defaultValue: "Waiting for cmux") | ||
| case .error(let message): | ||
| errorText = message |
There was a problem hiding this comment.
P3: Map known host error strings to localized text instead of displaying raw transport messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Examples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swift, line 28:
<comment>Map known host error strings to localized text instead of displaying raw transport messages.</comment>
<file context>
@@ -18,8 +18,15 @@ final class SidebarConnectionModel {
+ case .waitingForHost:
+ errorText = String(localized: "sampleSidebar.waitingForHost", defaultValue: "Waiting for cmux")
+ case .error(let message):
+ errorText = message
+ }
}
</file context>
Suggested change
| errorText = message | |
| errorText = message == "cmux did not send a workspace snapshot" | |
| ? String(localized: "sampleSidebar.emptySnapshot", defaultValue: "cmux did not send a workspace snapshot") | |
| : message |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift (1)
4-44: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSplit the three public types into their own files and rename to match.
This file declares three public types (
CmuxSidebarActionResult,CmuxSidebarActionRejectionReason,CmuxSidebarActionError), and the filenameCMUXExtensionActionResult.swiftno longer matches any of them after the rename.CmuxSidebarActionRejectionReasonandCmuxSidebarActionErrorare meaningful public types that warrant their own files.As per coding guidelines: "One major type per file; each struct/class/enum/actor/protocol in public API lives in own file named after type; small helpers and nested types stay with parent."
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift` around lines 4 - 44, This file contains three public API types (CmuxSidebarActionResult, CmuxSidebarActionRejectionReason, CmuxSidebarActionError) and must be split so each public type lives in its own file named after the type; create three files each declaring one of: CmuxSidebarActionResult, CmuxSidebarActionRejectionReason, and CmuxSidebarActionError (moving their definitions verbatim), keep any `@_spi` or access modifiers with the corresponding type, and update/remove the original CMUXExtensionActionResult.swift so no public type is left in a mismatched file name.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift (1)
8-26:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix potential self-retain cycle + missing
@unchecked Sendablejustification
CmuxSidebarExtensionRuntime’sonSnapshotclosure capturestransportstrongly (transport.perform/transport.refreshSnapshot) whileCMUXSidebarExtensionConnectionstoresonSnapshot/onStatusasletproperties; this creates a retain cycle whereCMUXSidebarExtensionConnectioncan keep itself alive, andinvalidate()only clears internal connection state (it does not release those handlers).CmuxSidebarExtensionRuntimeis@unchecked Sendablewithout the required safety justification comment.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift` around lines 8 - 26, The onSnapshot closure currently captures transport strongly (used in transport.perform and transport.refreshSnapshot), creating a retain cycle because CMUXSidebarExtensionConnection stores the handlers; change the closure captures to use a weak (or unowned if provably safe) reference to transport (e.g., [weak transport]) and guard/early-return if transport is nil before calling perform/refreshSnapshot so handlers don’t retain the connection; keep invalidate() behavior as-is but ensure handlers can break the cycle. Also add the required `@unchecked` Sendable justification comment on the CmuxSidebarExtensionRuntime type explaining why it's safe to mark unchecked (referencing thread-safety of captured state and that external synchronization or immutability guarantees are in place).
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift`:
- Line 3: Add a concise safety-argument comment explaining why using `@unchecked`
Sendable on the CmuxSidebarExtensionRuntime declaration is safe: locate the
class CmuxSidebarExtensionRuntime and append a comment on the same line or
immediately above the declaration that states the invariant (for example, what
internal state is being wrapped and the thread/actor controlling all mutations,
e.g., "Wraps X; all mutations occur on Y/are synchronized by Z"), making clear
there are no data races and why the unchecked annotation is justified.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderMutations.swift`:
- Around line 3-49: Add Swift-DocC triple-slash comments to every public symbol
in this file: document the enum CmuxSidebarProviderPresentationRequest and each
of its cases, the struct CmuxSidebarProviderWorkspaceMove and its initializer
and properties (workspaceId, sourceSectionId, targetSectionId, targetIndex), the
enum CmuxSidebarProviderMutation and each case, the struct
CmuxSidebarProviderCommandResult and its init + ok property, and the protocol
CmuxMutableSidebarProvider and its handle(_:snapshot:) method; ensure each
public type and public member has a brief /// description explaining purpose,
parameters, and return value where applicable to satisfy the package
documentation guideline.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderPresentation.swift`:
- Around line 3-12: Public enums CmuxSidebarProviderPresentation and
CmuxSidebarProviderWorkspacePopoverTab lack Swift-DocC comments; add
triple-slash (///) documentation above each public enum
(CmuxSidebarProviderPresentation, CmuxSidebarProviderWorkspacePopoverTab) and
above each case (tree, browserStack, notes, browser, pullRequest) describing
their purpose and any special semantics (e.g., rawValue "browser-stack"),
example usage or platform scope if relevant, and mark parameters/returns or
remarks only if applicable so the public symbols are fully documented per
package guidelines.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderRenderModel.swift`:
- Around line 3-49: Add Swift-DocC triple-slash comments to all public API in
this file: document the CmuxSidebarProviderRenderModel type and its public init
and stored properties (providerId, snapshotSequence, sections, presentation),
the CmuxSidebarProviderRenderContext type and its public init/now property, the
CmuxContextualSidebarProvider protocol and its render(snapshot:context:)
requirement, and the two public render(...) extension methods on
CmuxSidebarProvider; explicitly note in the documentation for the default
render(snapshot:context:) extension that it intentionally ignores the context
and delegates to render(snapshot:). Ensure each comment is concise, describes
purpose and behavior, and appears immediately above the corresponding public
symbol using triple-slash DocC format.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderRows.swift`:
- Around line 3-101: Public API symbols in CmuxSidebarProviderRows are
undocumented; add Swift-DocC triple-slash comments for all seven public types
and their public members/initializers. Specifically document
CmuxSidebarProviderRowAccessoryKind, CmuxSidebarProviderRowAccessory (and its
init and static inspector), CmuxSidebarProviderRelativeDateStyle,
CmuxSidebarProviderIconShape, CmuxSidebarProviderIcon (and its init),
CmuxSidebarProviderText (document cases and relativeDate property), and
CmuxSidebarProviderRow (and its init and public properties) with concise
summaries, parameter/return tags where applicable, and example/remarks if
helpful to satisfy the package guideline. Ensure every public stored property,
enum case, initializer, and computed property has a triple-slash comment
immediately above it.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSections.swift`:
- Around line 13-22: The initializer public init(...) has inconsistent optional
defaults: titleText and subtitleText default to nil but subtitle and
projectRootPath are required; update the initializer signature in
CmuxSidebarProviderSections.swift so that subtitle: String? and projectRootPath:
String? also have default values of nil (matching titleText/subtitleText) to
improve ergonomics when calling the init; keep the parameter order and names
(id, title, titleText, subtitle, subtitleText, systemImageName, projectRootPath,
workspaceIds) and adjust any call sites if necessary to rely on the new
defaults.
- Around line 3-48: The two public types CmuxSidebarProviderTreeSection and
CmuxSidebarProviderSection (including their public properties like id, title,
titleText, subtitle, subtitleText, systemImageName, projectRootPath,
workspaceIds, treeSection, rows and their public init initializers) must each be
moved into their own file (one major public type per file) and annotated with
Swift-DocC triple-slash comments; add concise /// documentation for each public
struct, every public property, and the public init parameters describing purpose
and semantics, then split the current file so CmuxSidebarProviderTreeSection is
defined in its own file and CmuxSidebarProviderSection in its own file to
satisfy the one-type-per-file guideline.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSnapshot.swift`:
- Around line 3-92: Split the three public types into their own files and add
Swift-DocC comments for each public symbol: create
CmuxSidebarProviderSnapshot.swift containing the CmuxSidebarProviderSnapshot
struct and document its properties and init with triple-slash comments; create
CmuxSidebarProviderGitBranch.swift for CmuxSidebarProviderGitBranch and document
branch/isDirty and init; create CmuxSidebarProviderWorkspace.swift for
CmuxSidebarProviderWorkspace and document the struct, all public properties (id,
title, customDescription, isPinned, rootPath, projectRootPath, branchSummary,
remoteDisplayTarget, remoteConnectionState, unreadCount, latestNotificationText,
latestSubmittedMessage, latestSubmittedAt, listeningPorts, pullRequestURLs,
panelDirectories, gitBranches) and its init; ensure file names match the type
names and that each public symbol has a brief Swift-DocC description above it.
---
Outside diff comments:
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`:
- Around line 4-44: This file contains three public API types
(CmuxSidebarActionResult, CmuxSidebarActionRejectionReason,
CmuxSidebarActionError) and must be split so each public type lives in its own
file named after the type; create three files each declaring one of:
CmuxSidebarActionResult, CmuxSidebarActionRejectionReason, and
CmuxSidebarActionError (moving their definitions verbatim), keep any `@_spi` or
access modifiers with the corresponding type, and update/remove the original
CMUXExtensionActionResult.swift so no public type is left in a mismatched file
name.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swift`:
- Around line 8-26: The onSnapshot closure currently captures transport strongly
(used in transport.perform and transport.refreshSnapshot), creating a retain
cycle because CMUXSidebarExtensionConnection stores the handlers; change the
closure captures to use a weak (or unowned if provably safe) reference to
transport (e.g., [weak transport]) and guard/early-return if transport is nil
before calling perform/refreshSnapshot so handlers don’t retain the connection;
keep invalidate() behavior as-is but ensure handlers can break the cycle. Also
add the required `@unchecked` Sendable justification comment on the
CmuxSidebarExtensionRuntime type explaining why it's safe to mark unchecked
(referencing thread-safety of captured state and that external synchronization
or immutability guarantees are in place).
🪄 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: edbbc48c-4ba5-40f6-854f-f2d21df5977d
📒 Files selected for processing (11)
Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarHost.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderMutations.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderPresentation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderRenderModel.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderRows.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSections.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSnapshot.swift
Comment on lines
+3
to
+48
| public struct CmuxSidebarProviderTreeSection: Identifiable, Codable, Equatable, Sendable { | ||
| public var id: String | ||
| public var title: String | ||
| public var titleText: CmuxSidebarProviderLocalizedText? | ||
| public var subtitle: String? | ||
| public var subtitleText: CmuxSidebarProviderLocalizedText? | ||
| public var systemImageName: String | ||
| public var projectRootPath: String? | ||
| public var workspaceIds: [UUID] | ||
|
|
||
| public init( | ||
| id: String, | ||
| title: String, | ||
| titleText: CmuxSidebarProviderLocalizedText? = nil, | ||
| subtitle: String?, | ||
| subtitleText: CmuxSidebarProviderLocalizedText? = nil, | ||
| systemImageName: String, | ||
| projectRootPath: String?, | ||
| workspaceIds: [UUID] | ||
| ) { | ||
| self.id = id | ||
| self.title = title | ||
| self.titleText = titleText | ||
| self.subtitle = subtitle | ||
| self.subtitleText = subtitleText | ||
| self.systemImageName = systemImageName | ||
| self.projectRootPath = projectRootPath | ||
| self.workspaceIds = workspaceIds | ||
| } | ||
| } | ||
|
|
||
| public struct CmuxSidebarProviderSection: Identifiable, Codable, Equatable, Sendable { | ||
| public var id: String | ||
| public var treeSection: CmuxSidebarProviderTreeSection | ||
| public var rows: [CmuxSidebarProviderRow] | ||
|
|
||
| public init( | ||
| id: String, | ||
| treeSection: CmuxSidebarProviderTreeSection, | ||
| rows: [CmuxSidebarProviderRow] | ||
| ) { | ||
| self.id = id | ||
| self.treeSection = treeSection | ||
| self.rows = rows | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document public symbols and split into one file per type.
Two issues for this new-package file:
- All public symbols (
CmuxSidebarProviderTreeSection,CmuxSidebarProviderSection, their properties and initializers) lack Swift-DocC comments. - The file declares two major public types.
As per coding guidelines: "Every public symbol in new Swift packages under Packages/ must be documented with Swift-DocC triple-slash comment at time of writing" and "One major type per file; each struct/class/enum/actor/protocol in public API lives in own file named after type."
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSections.swift`
around lines 3 - 48, The two public types CmuxSidebarProviderTreeSection and
CmuxSidebarProviderSection (including their public properties like id, title,
titleText, subtitle, subtitleText, systemImageName, projectRootPath,
workspaceIds, treeSection, rows and their public init initializers) must each be
moved into their own file (one major public type per file) and annotated with
Swift-DocC triple-slash comments; add concise /// documentation for each public
struct, every public property, and the public init parameters describing purpose
and semantics, then split the current file so CmuxSidebarProviderTreeSection is
defined in its own file and CmuxSidebarProviderSection in its own file to
satisfy the one-type-per-file guideline.
Comment on lines
+13
to
+22
| public init( | ||
| id: String, | ||
| title: String, | ||
| titleText: CmuxSidebarProviderLocalizedText? = nil, | ||
| subtitle: String?, | ||
| subtitleText: CmuxSidebarProviderLocalizedText? = nil, | ||
| systemImageName: String, | ||
| projectRootPath: String?, | ||
| workspaceIds: [UUID] | ||
| ) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Optional-parameter defaults are inconsistent.
titleText/subtitleText default to nil, but the equally-optional subtitle and projectRootPath require explicit arguments. Consider defaulting them for ergonomic call sites.
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSections.swift`
around lines 13 - 22, The initializer public init(...) has inconsistent optional
defaults: titleText and subtitleText default to nil but subtitle and
projectRootPath are required; update the initializer signature in
CmuxSidebarProviderSections.swift so that subtitle: String? and projectRootPath:
String? also have default values of nil (matching titleText/subtitleText) to
improve ergonomics when calling the init; keep the parameter order and names
(id, title, titleText, subtitle, subtitleText, systemImageName, projectRootPath,
workspaceIds) and adjust any call sites if necessary to rely on the new
defaults.
Comment on lines
+3
to
+92
| public struct CmuxSidebarProviderSnapshot: Codable, Equatable, Sendable { | ||
| public var sequence: UInt64 | ||
| public var selectedWorkspaceId: UUID? | ||
| public var workspaces: [CmuxSidebarProviderWorkspace] | ||
| public var windowId: UUID? | ||
|
|
||
| public init( | ||
| sequence: UInt64, | ||
| selectedWorkspaceId: UUID?, | ||
| workspaces: [CmuxSidebarProviderWorkspace], | ||
| windowId: UUID? = nil | ||
| ) { | ||
| self.sequence = sequence | ||
| self.selectedWorkspaceId = selectedWorkspaceId | ||
| self.workspaces = workspaces | ||
| self.windowId = windowId | ||
| } | ||
|
|
||
| public var workspaceIds: [UUID] { | ||
| workspaces.map(\.id) | ||
| } | ||
| } | ||
|
|
||
| public struct CmuxSidebarProviderGitBranch: Codable, Equatable, Sendable { | ||
| public var branch: String | ||
| public var isDirty: Bool | ||
|
|
||
| public init(branch: String, isDirty: Bool) { | ||
| self.branch = branch | ||
| self.isDirty = isDirty | ||
| } | ||
| } | ||
|
|
||
| public struct CmuxSidebarProviderWorkspace: Identifiable, Codable, Equatable, Sendable { | ||
| public var id: UUID | ||
| public var title: String | ||
| public var customDescription: String? | ||
| public var isPinned: Bool | ||
| public var rootPath: String? | ||
| public var projectRootPath: String? | ||
| public var branchSummary: String? | ||
| public var remoteDisplayTarget: String? | ||
| public var remoteConnectionState: String? | ||
| public var unreadCount: Int | ||
| public var latestNotificationText: String? | ||
| public var latestSubmittedMessage: String? | ||
| public var latestSubmittedAt: Date? | ||
| public var listeningPorts: [Int] | ||
| public var pullRequestURLs: [String] | ||
| public var panelDirectories: [String] | ||
| public var gitBranches: [CmuxSidebarProviderGitBranch] | ||
|
|
||
| public init( | ||
| id: UUID, | ||
| title: String, | ||
| customDescription: String?, | ||
| isPinned: Bool, | ||
| rootPath: String?, | ||
| projectRootPath: String?, | ||
| branchSummary: String?, | ||
| remoteDisplayTarget: String?, | ||
| remoteConnectionState: String?, | ||
| unreadCount: Int, | ||
| latestNotificationText: String?, | ||
| latestSubmittedMessage: String? = nil, | ||
| latestSubmittedAt: Date? = nil, | ||
| listeningPorts: [Int], | ||
| pullRequestURLs: [String] = [], | ||
| panelDirectories: [String] = [], | ||
| gitBranches: [CmuxSidebarProviderGitBranch] = [] | ||
| ) { | ||
| self.id = id | ||
| self.title = title | ||
| self.customDescription = customDescription | ||
| self.isPinned = isPinned | ||
| self.rootPath = rootPath | ||
| self.projectRootPath = projectRootPath | ||
| self.branchSummary = branchSummary | ||
| self.remoteDisplayTarget = remoteDisplayTarget | ||
| self.remoteConnectionState = remoteConnectionState | ||
| self.unreadCount = unreadCount | ||
| self.latestNotificationText = latestNotificationText | ||
| self.latestSubmittedMessage = latestSubmittedMessage | ||
| self.latestSubmittedAt = latestSubmittedAt | ||
| self.listeningPorts = listeningPorts | ||
| self.pullRequestURLs = pullRequestURLs | ||
| self.panelDirectories = panelDirectories | ||
| self.gitBranches = gitBranches | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document public symbols and split the three types into separate files.
This new-package file declares CmuxSidebarProviderSnapshot, CmuxSidebarProviderGitBranch, and CmuxSidebarProviderWorkspace with no Swift-DocC comments on any public symbol, and bundles three major public types in one file.
As per coding guidelines: "Every public symbol in new Swift packages under Packages/ must be documented with Swift-DocC triple-slash comment at time of writing" and "One major type per file; each struct/class/enum/actor/protocol in public API lives in own file named after type."
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderSnapshot.swift`
around lines 3 - 92, Split the three public types into their own files and add
Swift-DocC comments for each public symbol: create
CmuxSidebarProviderSnapshot.swift containing the CmuxSidebarProviderSnapshot
struct and document its properties and init with triple-slash comments; create
CmuxSidebarProviderGitBranch.swift for CmuxSidebarProviderGitBranch and document
branch/isDirty and init; create CmuxSidebarProviderWorkspace.swift for
CmuxSidebarProviderWorkspace and document the struct, all public properties (id,
title, customDescription, isPinned, rootPath, projectRootPath, branchSummary,
remoteDisplayTarget, remoteConnectionState, unreadCount, latestNotificationText,
latestSubmittedMessage, latestSubmittedAt, listeningPorts, pullRequestURLs,
panelDirectories, gitBranches) and its init; ensure file names match the type
names and that each public symbol has a brief Swift-DocC description above it.
There was a problem hiding this comment.
9 issues found across 26 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swift:3">
P3: Public API doc comment still uses legacy `CMUX` branding instead of `Cmux`, creating naming inconsistency in the new SDK surface.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swift:3">
P3: Doc comments still reference the legacy "CMUX" branding instead of the renamed "Cmux" prefix, inconsistent with the PR's stated global renaming from CMUX* → Cmux*.</violation>
<violation number="2" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swift:10">
P2: `openURL` case uses a bare `String` parameter instead of `URL`, sacrificing compile-time type safety for an API that semantically represents a URL. `URL` is `Codable` in Foundation, so this would not break the enum's `Codable` conformance.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift:25">
P3: The `accessory` init parameter is typed as optional (`CmuxSidebarProviderRowAccessory?`) but lacks a default value of `nil`, unlike the other three optional parameters (`subtitle`, `trailingText`, `leadingIcon`) which all default to `nil`. This forces callers to explicitly pass `nil` even when no accessory is needed, creating unnecessary friction and inconsistency.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swift:6">
P3: Render-model struct uses `var` instead of `let` for its stored properties, which allows external mutation after construction. For a `Sendable` value type that represents an immutable snapshot/emission of provider state, `let` properties better communicate the snapshot semantics and prevent accidental state corruption by consumers.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swift:18">
P1: The `render(snapshot:context:)` method on `CmuxSidebarProvider` ignores the `context` parameter and creates a static dispatch ambiguity with `CmuxContextualSidebarProvider`. Because this method is NOT a requirement of `CmuxSidebarProvider` (it only exists in the protocol extension), Swift uses static dispatch when calling through a `CmuxSidebarProvider`-typed reference. A caller with a `CmuxSidebarProvider` reference pointing to an object that actually conforms to `CmuxContextualSidebarProvider` will silently get this context-ignoring default instead of the contextual implementation, causing the context (e.g., `context.now` for relative date formatting) to be discarded without warning.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift:16">
P2: `defaultTab` is a required property in the initializer, but its semantics ("popover tab when the accessory opens workspace details") are only meaningful for certain accessory kinds. Currently `CmuxSidebarProviderRowAccessoryKind` has a single case (`.workspaceInspector`), which happens to open workspace details, so this works today. However, as soon as another kind is added (e.g., a direct-action button, a context-menu trigger), callers must supply a meaningless `defaultTab` just to construct the struct. This couples unrelated concerns into a single required parameter.</violation>
</file>
<file name="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swift">
<violation number="1" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swift:6">
P2: Icon model allows an empty/unrenderable state: both `systemImageName` and `text` are optional with no guard ensuring at least one is non-nil. Since `leadingIcon` on the row is already `CmuxSidebarProviderIcon?`, a nil icon already means "no icon"; an icon struct with neither field set is a meaningless state that should be prevented at the type level.</violation>
<violation number="2" location="Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swift:10">
P2: Hex color strings accept any arbitrary `String?` with no format validation, despite being documented as "CSS-style hex string." Invalid values (wrong length, missing `#`, invalid characters) would be silently accepted and could cause rendering failures downstream.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| snapshot: CmuxSidebarProviderSnapshot, | ||
| context: CmuxSidebarProviderRenderContext | ||
| ) -> CmuxSidebarProviderRenderModel { | ||
| render(snapshot: snapshot) |
There was a problem hiding this comment.
P1: The render(snapshot:context:) method on CmuxSidebarProvider ignores the context parameter and creates a static dispatch ambiguity with CmuxContextualSidebarProvider. Because this method is NOT a requirement of CmuxSidebarProvider (it only exists in the protocol extension), Swift uses static dispatch when calling through a CmuxSidebarProvider-typed reference. A caller with a CmuxSidebarProvider reference pointing to an object that actually conforms to CmuxContextualSidebarProvider will silently get this context-ignoring default instead of the contextual implementation, causing the context (e.g., context.now for relative date formatting) to be discarded without warning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swift, line 18:
<comment>The `render(snapshot:context:)` method on `CmuxSidebarProvider` ignores the `context` parameter and creates a static dispatch ambiguity with `CmuxContextualSidebarProvider`. Because this method is NOT a requirement of `CmuxSidebarProvider` (it only exists in the protocol extension), Swift uses static dispatch when calling through a `CmuxSidebarProvider`-typed reference. A caller with a `CmuxSidebarProvider` reference pointing to an object that actually conforms to `CmuxContextualSidebarProvider` will silently get this context-ignoring default instead of the contextual implementation, causing the context (e.g., `context.now` for relative date formatting) to be discarded without warning.</comment>
<file context>
@@ -0,0 +1,20 @@
+ snapshot: CmuxSidebarProviderSnapshot,
+ context: CmuxSidebarProviderRenderContext
+ ) -> CmuxSidebarProviderRenderModel {
+ render(snapshot: snapshot)
+ }
+}
</file context>
| /// Open a detached workspace window on a preferred tab. | ||
| case openWorkspaceWindow(workspaceId: UUID, preferredTab: CmuxSidebarProviderWorkspacePopoverTab) | ||
| /// Ask CMUX to open a URL. | ||
| case openURL(String) |
There was a problem hiding this comment.
P2: openURL case uses a bare String parameter instead of URL, sacrificing compile-time type safety for an API that semantically represents a URL. URL is Codable in Foundation, so this would not break the enum's Codable conformance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swift, line 10:
<comment>`openURL` case uses a bare `String` parameter instead of `URL`, sacrificing compile-time type safety for an API that semantically represents a URL. `URL` is `Codable` in Foundation, so this would not break the enum's `Codable` conformance.</comment>
<file context>
@@ -0,0 +1,11 @@
+ /// Open a detached workspace window on a preferred tab.
+ case openWorkspaceWindow(workspaceId: UUID, preferredTab: CmuxSidebarProviderWorkspacePopoverTab)
+ /// Ask CMUX to open a URL.
+ case openURL(String)
+}
</file context>
| public init( | ||
| kind: CmuxSidebarProviderRowAccessoryKind, | ||
| systemImageName: String, | ||
| defaultTab: CmuxSidebarProviderWorkspacePopoverTab |
There was a problem hiding this comment.
P2: defaultTab is a required property in the initializer, but its semantics ("popover tab when the accessory opens workspace details") are only meaningful for certain accessory kinds. Currently CmuxSidebarProviderRowAccessoryKind has a single case (.workspaceInspector), which happens to open workspace details, so this works today. However, as soon as another kind is added (e.g., a direct-action button, a context-menu trigger), callers must supply a meaningless defaultTab just to construct the struct. This couples unrelated concerns into a single required parameter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift, line 16:
<comment>`defaultTab` is a required property in the initializer, but its semantics ("popover tab when the accessory opens workspace details") are only meaningful for certain accessory kinds. Currently `CmuxSidebarProviderRowAccessoryKind` has a single case (`.workspaceInspector`), which happens to open workspace details, so this works today. However, as soon as another kind is added (e.g., a direct-action button, a context-menu trigger), callers must supply a meaningless `defaultTab` just to construct the struct. This couples unrelated concerns into a single required parameter.</comment>
<file context>
@@ -0,0 +1,29 @@
+ public init(
+ kind: CmuxSidebarProviderRowAccessoryKind,
+ systemImageName: String,
+ defaultTab: CmuxSidebarProviderWorkspacePopoverTab
+ ) {
+ self.kind = kind
</file context>
| /// Optional short text fallback. | ||
| public var text: String? | ||
| /// Foreground color as a CSS-style hex string. | ||
| public var foregroundColorHex: String? |
There was a problem hiding this comment.
P2: Hex color strings accept any arbitrary String? with no format validation, despite being documented as "CSS-style hex string." Invalid values (wrong length, missing #, invalid characters) would be silently accepted and could cause rendering failures downstream.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swift, line 10:
<comment>Hex color strings accept any arbitrary `String?` with no format validation, despite being documented as "CSS-style hex string." Invalid values (wrong length, missing `#`, invalid characters) would be silently accepted and could cause rendering failures downstream.</comment>
<file context>
@@ -0,0 +1,30 @@
+ /// Optional short text fallback.
+ public var text: String?
+ /// Foreground color as a CSS-style hex string.
+ public var foregroundColorHex: String?
+ /// Background color as a CSS-style hex string.
+ public var backgroundColorHex: String?
</file context>
| /// Icon model for a provider row. | ||
| public struct CmuxSidebarProviderIcon: Codable, Equatable, Sendable { | ||
| /// Optional SF Symbols name. | ||
| public var systemImageName: String? |
There was a problem hiding this comment.
P2: Icon model allows an empty/unrenderable state: both systemImageName and text are optional with no guard ensuring at least one is non-nil. Since leadingIcon on the row is already CmuxSidebarProviderIcon?, a nil icon already means "no icon"; an icon struct with neither field set is a meaningless state that should be prevented at the type level.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swift, line 6:
<comment>Icon model allows an empty/unrenderable state: both `systemImageName` and `text` are optional with no guard ensuring at least one is non-nil. Since `leadingIcon` on the row is already `CmuxSidebarProviderIcon?`, a nil icon already means "no icon"; an icon struct with neither field set is a meaningless state that should be prevented at the type level.</comment>
<file context>
@@ -0,0 +1,30 @@
+/// Icon model for a provider row.
+public struct CmuxSidebarProviderIcon: Codable, Equatable, Sendable {
+ /// Optional SF Symbols name.
+ public var systemImageName: String?
+ /// Optional short text fallback.
+ public var text: String?
</file context>
| @@ -0,0 +1,31 @@ | |||
| import Foundation | |||
|
|
|||
| /// Snapshot of CMUX workspace state consumed by in-process sidebar providers. | |||
There was a problem hiding this comment.
P3: Public API doc comment still uses legacy CMUX branding instead of Cmux, creating naming inconsistency in the new SDK surface.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swift, line 3:
<comment>Public API doc comment still uses legacy `CMUX` branding instead of `Cmux`, creating naming inconsistency in the new SDK surface.</comment>
<file context>
@@ -0,0 +1,31 @@
+import Foundation
+
+/// Snapshot of CMUX workspace state consumed by in-process sidebar providers.
+public struct CmuxSidebarProviderSnapshot: Codable, Equatable, Sendable {
+ /// Monotonic snapshot sequence.
</file context>
Suggested change
| /// Snapshot of CMUX workspace state consumed by in-process sidebar providers. | |
| /// Snapshot of Cmux workspace state consumed by in-process sidebar providers. |
| @@ -0,0 +1,11 @@ | |||
| import Foundation | |||
|
|
|||
| /// Presentation command a provider can request from the CMUX sidebar host. | |||
There was a problem hiding this comment.
P3: Doc comments still reference the legacy "CMUX" branding instead of the renamed "Cmux" prefix, inconsistent with the PR's stated global renaming from CMUX* → Cmux*.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swift, line 3:
<comment>Doc comments still reference the legacy "CMUX" branding instead of the renamed "Cmux" prefix, inconsistent with the PR's stated global renaming from CMUX* → Cmux*.</comment>
<file context>
@@ -0,0 +1,11 @@
+import Foundation
+
+/// Presentation command a provider can request from the CMUX sidebar host.
+public enum CmuxSidebarProviderPresentationRequest: Codable, Equatable, Sendable {
+ /// Open the workspace popover on a preferred tab.
</file context>
Suggested change
| /// Presentation command a provider can request from the CMUX sidebar host. | |
| /// Presentation command a provider can request from the Cmux sidebar host. |
| id: UUID, | ||
| title: String, | ||
| workspaceId: UUID, | ||
| accessory: CmuxSidebarProviderRowAccessory?, |
There was a problem hiding this comment.
P3: The accessory init parameter is typed as optional (CmuxSidebarProviderRowAccessory?) but lacks a default value of nil, unlike the other three optional parameters (subtitle, trailingText, leadingIcon) which all default to nil. This forces callers to explicitly pass nil even when no accessory is needed, creating unnecessary friction and inconsistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift, line 25:
<comment>The `accessory` init parameter is typed as optional (`CmuxSidebarProviderRowAccessory?`) but lacks a default value of `nil`, unlike the other three optional parameters (`subtitle`, `trailingText`, `leadingIcon`) which all default to `nil`. This forces callers to explicitly pass `nil` even when no accessory is needed, creating unnecessary friction and inconsistency.</comment>
<file context>
@@ -0,0 +1,38 @@
+ id: UUID,
+ title: String,
+ workspaceId: UUID,
+ accessory: CmuxSidebarProviderRowAccessory?,
+ subtitle: CmuxSidebarProviderText? = nil,
+ trailingText: CmuxSidebarProviderText? = nil,
</file context>
| /// Complete render model emitted by an in-process sidebar provider. | ||
| public struct CmuxSidebarProviderRenderModel: Codable, Equatable, Sendable { | ||
| /// Provider id that produced this model. | ||
| public var providerId: String |
There was a problem hiding this comment.
P3: Render-model struct uses var instead of let for its stored properties, which allows external mutation after construction. For a Sendable value type that represents an immutable snapshot/emission of provider state, let properties better communicate the snapshot semantics and prevent accidental state corruption by consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swift, line 6:
<comment>Render-model struct uses `var` instead of `let` for its stored properties, which allows external mutation after construction. For a `Sendable` value type that represents an immutable snapshot/emission of provider state, `let` properties better communicate the snapshot semantics and prevent accidental state corruption by consumers.</comment>
<file context>
@@ -0,0 +1,26 @@
+/// Complete render model emitted by an in-process sidebar provider.
+public struct CmuxSidebarProviderRenderModel: Codable, Equatable, Sendable {
+ /// Provider id that produced this model.
+ public var providerId: String
+ /// Snapshot sequence this model was rendered from.
+ public var snapshotSequence: UInt64
</file context>
azooz2003-bit
force-pushed
the
feat-extension-sdk-organization
branch
from
June 2, 2026 14:35
ba6476f to
3c778bf
Compare
azooz2003-bit
force-pushed
the
feat-extension-sdk-organization
branch
from
June 2, 2026 14:47
3c778bf to
8173919
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift (1)
37-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix missing translations for
defaultWorkspacessidebar provider title/subtitle
defaultWorkspacesusesCmuxSidebarProviderLocalizedTextforsidebar.provider.default.titleandsidebar.provider.default.subtitle, but inResources/Localizable.xcstringsthose keys are localized only forenandjawhile the catalog includes many other supported locales (e.g.,fr,de,es,it,zh-Hans,zh-Hant, etc.). This will fall back to English for the other locales, violating the full internationalization rule.🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift` around lines 37 - 38, The title/subtitle for the defaultWorkspaces provider (created with CmuxSidebarProviderLocalizedText in CmuxSidebarProviderDescriptor.swift using keys "sidebar.provider.default.title" and "sidebar.provider.default.subtitle") are only localized for en/ja; add the missing translations for all supported locales in Resources/Localizable.xcstrings (e.g., fr, de, es, it, zh-Hans, zh-Hant, etc.) so these keys exist in each locale bundle, or update the localization catalog to include those keys for every supported locale to avoid English fallbacks.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift (1)
3-42: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExpand the DocC on this public tree-section API.
This file adds a public package model, but its docs are still summary-only. The repo rule for
Packages/requires full Swift-DocC coverage for the type, public properties, andinit(...), including discussion text and parameter callouts.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "document enums/inits/properties/methods with meaning and invariants."🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift` around lines 3 - 42, Add full Swift-DocC documentation to the public API: provide a triple-slash doc comment for the CmuxSidebarProviderTreeSection type describing its purpose and invariants, then add triple-slash comments for each public property (id, title, titleText, subtitle, subtitleText, systemImageName, projectRootPath, workspaceIds) explaining meaning, expected formats (e.g., stable id, SF Symbols name), nilability, and any invariants (e.g., workspaceIds non-empty if representing a workspace). Also document the public init(...) with a DocC discussion and `@param-style` callouts for each parameter describing allowed values and behavior (including defaults for optional parameters) and note thread-safety/Sendable conformance where relevant.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift (1)
101-104: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConsider using localized strings for transport error messages.
Multiple hardcoded English strings (
"Waiting for cmux","cmux connection changed","cmux connection was lost","cmux did not send a workspace snapshot", etc.) flow through rejection results and may surface in extension UIs.The downstream sample in
SidebarConnectionModel.swiftshows these can becomeerrorText. While these are in internal transport code, consider either:
- Documenting that host rejection messages are diagnostic-only and extensions should provide their own localized fallback UI, or
- Using localized strings with
Bundle.modulefor consistency.Current sample code handles this appropriately by mapping
.rejected(message)to its own localized fallback, so this is not blocking.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift` around lines 101 - 104, Replace hardcoded English transport messages (e.g. the "Waiting for cmux" message passed into deliver(.rejected(message), to: reply) and other strings used when reporting connection state like in report(.waitingForHost) or deliver(.rejected(...))) with localized strings from your package bundle (e.g. use NSLocalizedString or a localized helper that loads from Bundle.module) so these rejection messages can be localized or documented as diagnostic-only; update the message creation in CMUXSidebarExtensionConnection (the spots that call report(.waitingForHost, ifCurrentGeneration: currentGeneration()) and deliver(.rejected(...), to: reply)) to fetch a localized string or a clearly documented diagnostic message provider.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift (1)
37-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHardcoded English string violates localization guidelines.
The
cancelledresult contains the hardcoded message"Extension action was cancelled"which may surface in extension UIs (seeSidebarConnectionModel.swiftcontext showingerrorText = message). Per full-internationalization.md, user-facing text must use localized APIs.Since this is inside a package (not the main app target), you'll need to use the
Bundle.modulepattern:📝 Proposed fix
/// Rejected action result used when the caller cancels an in-flight request. public static let cancelled = CmuxSidebarActionResult( accepted: false, - message: "Extension action was cancelled", + message: String( + localized: "sidebar.action.cancelled", + defaultValue: "Extension action was cancelled", + bundle: .module + ), rejectionReason: .cancelled )You'll also need to add a
Localizable.xcstringsresource to the package if one doesn't exist.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift` around lines 37 - 42, The static constant cancelled on CmuxSidebarActionResult currently embeds a hardcoded English message; replace the literal "Extension action was cancelled" with a localized lookup using Bundle.module (e.g., NSLocalizedString or String(localized:bundle:)) so the user-facing text comes from the package resource bundle, and add the corresponding key/value to the package's Localizable.xcstrings resource; update CmuxSidebarActionResult.cancelled to call the localization API with Bundle.module and ensure the Localizable.xcstrings contains the translated string for that key.
🤖 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
`@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swift`:
- Around line 46-52: The test currently loops "for provider in
SidebarExamples.providers where
listingProviderIDs.contains(provider.descriptor.id)" and can vacuously pass if
no providers match; update the test to assert that at least one provider was
exercised by either (a) computing the matchedProviders via
SidebarExamples.providers.filter { listingProviderIDs.contains($0.descriptor.id)
} and XCTAssertGreaterThan(matchedProviders.count, 0) before iterating, or (b)
keep the loop but increment a counter for each iteration and
XCTAssertGreaterThan(counter, 0) after the loop; use the same
provider.descriptor.id and model.sections.flatMap(\\.rows) checks inside the
iteration as-is.
- Around line 1-53: Convert this XCTest-based file to Swift Testing: replace
"import XCTest" with "import Testing", change the XCTestCase class
SidebarProviderExistentialDispatchTests into a Swift Testing suite (e.g.
annotate a struct or enum with `@Suite` and keep the same name), mark the two test
methods testSuperCompactRendersIdenticallyThroughExistential and
testWorkspaceListingProvidersRenderRowsThroughExistential with `@Test`, and
replace XCTAssert* calls with the corresponding `#expect`(...) matchers (e.g.
`#expect`(concreteModel.sections.isEmpty).toBeFalse(),
`#expect`(concreteModel.sections.flatMap(\.rows).count).toEqual(3), and
`#expect`(existentialModel.sections).toEqual(concreteModel.sections); similarly
use `#expect`(...).not.toBeEmpty() or equivalent for the provider loop). Keep
references to SuperCompactSidebar, SidebarExamples.providers,
provider.descriptor.id, and CmuxSidebarProvider render(snapshot:) usage
unchanged except for assertion syntax and suite/test annotations.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swift`:
- Around line 3-10: The public type CmuxSidebarProviderCommandResult and its
initializer init(ok:) lack complete DocC comments: update the triple-slash docs
for the struct, the public var ok and the public init(ok:) to include a summary
sentence for the type, a clear description of the ok property, and a "-
Parameter ok:" callout that explains what true and false mean for callers (e.g.,
ok == true means CMUX accepted and completed the command; ok == false means the
command was rejected or failed and callers should treat the result as
unsuccessful). Ensure comments are placed immediately above
CmuxSidebarProviderCommandResult, above the ok property, and above init(ok:)
using Swift-DocC triple-slash style.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swift`:
- Around line 11-24: The public transport type allows negative targetIndex
values which can be decoded or constructed; fix by enforcing a non-negative
invariant at decode-time or via a refined type: add a custom Decodable
init(from:) on CmuxSidebarProviderWorkspaceMove that reads targetIndex and
throws a decoding error if value < 0 (or replace targetIndex with a
NonNegativeInt wrapper type that validates on init/Decodable), and update the
public init/workflow to accept/produce only the validated value so negatives
cannot flow through the API.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift`:
- Around line 3-38: Add full Swift-DocC comments for the public type
CmuxSidebarProviderRow, its public stored properties (id, title, workspaceId,
accessory, subtitle, trailingText, leadingIcon) and the public initializer; for
each symbol use triple-slash comments with a one-sentence summary on the first
line, a blank line, then a short discussion paragraph, and for the init include
a detailed "- Parameter" entry for every parameter (and "- Returns:" only if
applicable) following DocC style; update the existing short summaries above the
struct and above init(...) to the required summary/discussion shape and add
per-parameter callouts to the init signature so the package meets the Packages/
Swift-DocC documentation policy.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift`:
- Around line 3-29: Add full Swift-DocC triple-slash documentation to the public
struct CmuxSidebarProviderRowAccessory, its public stored properties (kind,
systemImageName, defaultTab), the public initializer
(init(kind:systemImageName:defaultTab:)) and the public static inspector
constant: expand the brief summaries into a discussion paragraph describing
purpose and usage, document invariants/behavior, and add - Parameter callouts
for each initializer parameter explaining expected values and effects (e.g.,
what each CmuxSidebarProviderRowAccessoryKind value does, valid systemImageName
expectations, and defaultTab behavior); ensure the inspector constant has its
own summary/discussion noting it is a standard preconfigured accessory.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swift`:
- Around line 3-7: Add full Swift-DocC triple-slash documentation for the
exported enum CmuxSidebarProviderRowAccessoryKind and its public member(s):
document the enum's purpose, when and where clients should use it, any
invariants or expectations (e.g., that it is Codable/Equatable/Sendable and
backed by a String raw value), and describe serialization/compatibility
guarantees; for the case workspaceInspector add a clear sentence about what UI
affordance it represents and any behavior/constraints (for example whether it
always opens the workspace inspector, platform limits, or visibility rules).
Place the comments immediately above the enum declaration and above the case
using triple-slash (///) DocC style so the symbol and member are fully
documented for clients and documentation generation.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swift`:
- Around line 3-22: Add full Swift-DocC triple-slash documentation to the public
type CmuxSidebarProviderSection, each public property (id, treeSection, rows),
and the public initializer init(id:treeSection:rows:). For each symbol follow
the package guideline: start with a one-sentence summary on the first line, add
a blank /// line, then a short discussion paragraph describing purpose/behavior
and any invariants; for the initializer include a /// - Parameters: block
documenting id, treeSection, and rows. Ensure comments are triple-slash (///)
and placed immediately above the struct, each property, and the initializer so
DocC picks them up.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swift`:
- Around line 3-15: Add full Swift-DocC triple-slash documentation for the
public symbol CmuxSidebarProviderGitBranch: start with a one-sentence summary on
the first /// line, add a blank /// line, then a short discussion describing the
purpose and usage of this snapshot type; document the stored properties branch
and isDirty with brief descriptions; and add a documented initializer
init(branch:isDirty:) that includes /// - Parameters: with callouts for branch
and isDirty. Ensure every public symbol (struct, properties, and init) uses the
/// pattern and follows the package doc style.
In
`@Packages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swift`:
- Around line 25-28: Replace the hardcoded legacy keychain service/account
strings with the defined constants: use legacyKeychainService and
legacyKeychainAccount wherever the CFDictionary keychain queries are built (the
dictionaries currently containing "com.cmuxterm.app.socket-control" and
"local-socket-password"), including in the migration routine and
deleteLegacyPasswordFromKeychain(); update the query dictionaries to reference
these constants instead of string literals so the service/account names are
maintained in one place.
In `@Sources/ContentView.swift`:
- Around line 10356-10361: The view currently calls GhosttyConfig.load()
synchronously inside the SidebarTabItemSettingsStore initializer which may block
view init; change to construct SidebarTabItemSettingsStore with a fast default
(e.g. a default font size) and remove the synchronous GhosttyConfig.load() call,
then kick off an async load (using SidebarFontSizeProvider.loadFromGhosttyConfig
or a .task/async init helper) to fetch the real value and update the
SidebarTabItemSettingsStore instance when ready; target the
SidebarTabItemSettingsStore creation site and the async loader
SidebarFontSizeProvider.loadFromGhosttyConfig to implement the deferred load and
update logic.
---
Outside diff comments:
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`:
- Around line 37-42: The static constant cancelled on CmuxSidebarActionResult
currently embeds a hardcoded English message; replace the literal "Extension
action was cancelled" with a localized lookup using Bundle.module (e.g.,
NSLocalizedString or String(localized:bundle:)) so the user-facing text comes
from the package resource bundle, and add the corresponding key/value to the
package's Localizable.xcstrings resource; update
CmuxSidebarActionResult.cancelled to call the localization API with
Bundle.module and ensure the Localizable.xcstrings contains the translated
string for that key.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift`:
- Around line 101-104: Replace hardcoded English transport messages (e.g. the
"Waiting for cmux" message passed into deliver(.rejected(message), to: reply)
and other strings used when reporting connection state like in
report(.waitingForHost) or deliver(.rejected(...))) with localized strings from
your package bundle (e.g. use NSLocalizedString or a localized helper that loads
from Bundle.module) so these rejection messages can be localized or documented
as diagnostic-only; update the message creation in
CMUXSidebarExtensionConnection (the spots that call report(.waitingForHost,
ifCurrentGeneration: currentGeneration()) and deliver(.rejected(...), to:
reply)) to fetch a localized string or a clearly documented diagnostic message
provider.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift`:
- Around line 37-38: The title/subtitle for the defaultWorkspaces provider
(created with CmuxSidebarProviderLocalizedText in
CmuxSidebarProviderDescriptor.swift using keys "sidebar.provider.default.title"
and "sidebar.provider.default.subtitle") are only localized for en/ja; add the
missing translations for all supported locales in
Resources/Localizable.xcstrings (e.g., fr, de, es, it, zh-Hans, zh-Hant, etc.)
so these keys exist in each locale bundle, or update the localization catalog to
include those keys for every supported locale to avoid English fallbacks.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift`:
- Around line 3-42: Add full Swift-DocC documentation to the public API: provide
a triple-slash doc comment for the CmuxSidebarProviderTreeSection type
describing its purpose and invariants, then add triple-slash comments for each
public property (id, title, titleText, subtitle, subtitleText, systemImageName,
projectRootPath, workspaceIds) explaining meaning, expected formats (e.g.,
stable id, SF Symbols name), nilability, and any invariants (e.g., workspaceIds
non-empty if representing a workspace). Also document the public init(...) with
a DocC discussion and `@param-style` callouts for each parameter describing
allowed values and behavior (including defaults for optional parameters) and
note thread-safety/Sendable conformance where relevant.
🪄 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: 4e072668-a9ca-4dc2-a69d-bbc9583bb3cd
📒 Files selected for processing (46)
Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swiftExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/Resources/Localizable.xcstringsExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidator.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIconShape.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxMutableSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderMutation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderPresentation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderWorkspacePopoverTab.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxContextualSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderContext.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderWorkspace.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderRelativeDateStyle.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderText.swiftPackages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swiftResources/Localizable.xcstringsSources/ContentView.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatascripts/write-sidebar-extension-point.sh
💤 Files with no reviewable changes (2)
- scripts/write-sidebar-extension-point.sh
- Resources/Localizable.xcstrings
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift (1)
37-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix missing translations for
defaultWorkspacessidebar provider title/subtitle
defaultWorkspacesusesCmuxSidebarProviderLocalizedTextforsidebar.provider.default.titleandsidebar.provider.default.subtitle, but inResources/Localizable.xcstringsthose keys are localized only forenandjawhile the catalog includes many other supported locales (e.g.,fr,de,es,it,zh-Hans,zh-Hant, etc.). This will fall back to English for the other locales, violating the full internationalization rule.🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift` around lines 37 - 38, The title/subtitle for the defaultWorkspaces provider (created with CmuxSidebarProviderLocalizedText in CmuxSidebarProviderDescriptor.swift using keys "sidebar.provider.default.title" and "sidebar.provider.default.subtitle") are only localized for en/ja; add the missing translations for all supported locales in Resources/Localizable.xcstrings (e.g., fr, de, es, it, zh-Hans, zh-Hant, etc.) so these keys exist in each locale bundle, or update the localization catalog to include those keys for every supported locale to avoid English fallbacks.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift (1)
3-42: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winExpand the DocC on this public tree-section API.
This file adds a public package model, but its docs are still summary-only. The repo rule for
Packages/requires full Swift-DocC coverage for the type, public properties, andinit(...), including discussion text and parameter callouts.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "document enums/inits/properties/methods with meaning and invariants."🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift` around lines 3 - 42, Add full Swift-DocC documentation to the public API: provide a triple-slash doc comment for the CmuxSidebarProviderTreeSection type describing its purpose and invariants, then add triple-slash comments for each public property (id, title, titleText, subtitle, subtitleText, systemImageName, projectRootPath, workspaceIds) explaining meaning, expected formats (e.g., stable id, SF Symbols name), nilability, and any invariants (e.g., workspaceIds non-empty if representing a workspace). Also document the public init(...) with a DocC discussion and `@param-style` callouts for each parameter describing allowed values and behavior (including defaults for optional parameters) and note thread-safety/Sendable conformance where relevant.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift (1)
101-104: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConsider using localized strings for transport error messages.
Multiple hardcoded English strings (
"Waiting for cmux","cmux connection changed","cmux connection was lost","cmux did not send a workspace snapshot", etc.) flow through rejection results and may surface in extension UIs.The downstream sample in
SidebarConnectionModel.swiftshows these can becomeerrorText. While these are in internal transport code, consider either:
- Documenting that host rejection messages are diagnostic-only and extensions should provide their own localized fallback UI, or
- Using localized strings with
Bundle.modulefor consistency.Current sample code handles this appropriately by mapping
.rejected(message)to its own localized fallback, so this is not blocking.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift` around lines 101 - 104, Replace hardcoded English transport messages (e.g. the "Waiting for cmux" message passed into deliver(.rejected(message), to: reply) and other strings used when reporting connection state like in report(.waitingForHost) or deliver(.rejected(...))) with localized strings from your package bundle (e.g. use NSLocalizedString or a localized helper that loads from Bundle.module) so these rejection messages can be localized or documented as diagnostic-only; update the message creation in CMUXSidebarExtensionConnection (the spots that call report(.waitingForHost, ifCurrentGeneration: currentGeneration()) and deliver(.rejected(...), to: reply)) to fetch a localized string or a clearly documented diagnostic message provider.Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift (1)
37-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHardcoded English string violates localization guidelines.
The
cancelledresult contains the hardcoded message"Extension action was cancelled"which may surface in extension UIs (seeSidebarConnectionModel.swiftcontext showingerrorText = message). Per full-internationalization.md, user-facing text must use localized APIs.Since this is inside a package (not the main app target), you'll need to use the
Bundle.modulepattern:📝 Proposed fix
/// Rejected action result used when the caller cancels an in-flight request. public static let cancelled = CmuxSidebarActionResult( accepted: false, - message: "Extension action was cancelled", + message: String( + localized: "sidebar.action.cancelled", + defaultValue: "Extension action was cancelled", + bundle: .module + ), rejectionReason: .cancelled )You'll also need to add a
Localizable.xcstringsresource to the package if one doesn't exist.🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift` around lines 37 - 42, The static constant cancelled on CmuxSidebarActionResult currently embeds a hardcoded English message; replace the literal "Extension action was cancelled" with a localized lookup using Bundle.module (e.g., NSLocalizedString or String(localized:bundle:)) so the user-facing text comes from the package resource bundle, and add the corresponding key/value to the package's Localizable.xcstrings resource; update CmuxSidebarActionResult.cancelled to call the localization API with Bundle.module and ensure the Localizable.xcstrings contains the translated string for that key.
🤖 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
`@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swift`:
- Around line 46-52: The test currently loops "for provider in
SidebarExamples.providers where
listingProviderIDs.contains(provider.descriptor.id)" and can vacuously pass if
no providers match; update the test to assert that at least one provider was
exercised by either (a) computing the matchedProviders via
SidebarExamples.providers.filter { listingProviderIDs.contains($0.descriptor.id)
} and XCTAssertGreaterThan(matchedProviders.count, 0) before iterating, or (b)
keep the loop but increment a counter for each iteration and
XCTAssertGreaterThan(counter, 0) after the loop; use the same
provider.descriptor.id and model.sections.flatMap(\\.rows) checks inside the
iteration as-is.
- Around line 1-53: Convert this XCTest-based file to Swift Testing: replace
"import XCTest" with "import Testing", change the XCTestCase class
SidebarProviderExistentialDispatchTests into a Swift Testing suite (e.g.
annotate a struct or enum with `@Suite` and keep the same name), mark the two test
methods testSuperCompactRendersIdenticallyThroughExistential and
testWorkspaceListingProvidersRenderRowsThroughExistential with `@Test`, and
replace XCTAssert* calls with the corresponding `#expect`(...) matchers (e.g.
`#expect`(concreteModel.sections.isEmpty).toBeFalse(),
`#expect`(concreteModel.sections.flatMap(\.rows).count).toEqual(3), and
`#expect`(existentialModel.sections).toEqual(concreteModel.sections); similarly
use `#expect`(...).not.toBeEmpty() or equivalent for the provider loop). Keep
references to SuperCompactSidebar, SidebarExamples.providers,
provider.descriptor.id, and CmuxSidebarProvider render(snapshot:) usage
unchanged except for assertion syntax and suite/test annotations.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swift`:
- Around line 3-10: The public type CmuxSidebarProviderCommandResult and its
initializer init(ok:) lack complete DocC comments: update the triple-slash docs
for the struct, the public var ok and the public init(ok:) to include a summary
sentence for the type, a clear description of the ok property, and a "-
Parameter ok:" callout that explains what true and false mean for callers (e.g.,
ok == true means CMUX accepted and completed the command; ok == false means the
command was rejected or failed and callers should treat the result as
unsuccessful). Ensure comments are placed immediately above
CmuxSidebarProviderCommandResult, above the ok property, and above init(ok:)
using Swift-DocC triple-slash style.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swift`:
- Around line 11-24: The public transport type allows negative targetIndex
values which can be decoded or constructed; fix by enforcing a non-negative
invariant at decode-time or via a refined type: add a custom Decodable
init(from:) on CmuxSidebarProviderWorkspaceMove that reads targetIndex and
throws a decoding error if value < 0 (or replace targetIndex with a
NonNegativeInt wrapper type that validates on init/Decodable), and update the
public init/workflow to accept/produce only the validated value so negatives
cannot flow through the API.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift`:
- Around line 3-38: Add full Swift-DocC comments for the public type
CmuxSidebarProviderRow, its public stored properties (id, title, workspaceId,
accessory, subtitle, trailingText, leadingIcon) and the public initializer; for
each symbol use triple-slash comments with a one-sentence summary on the first
line, a blank line, then a short discussion paragraph, and for the init include
a detailed "- Parameter" entry for every parameter (and "- Returns:" only if
applicable) following DocC style; update the existing short summaries above the
struct and above init(...) to the required summary/discussion shape and add
per-parameter callouts to the init signature so the package meets the Packages/
Swift-DocC documentation policy.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift`:
- Around line 3-29: Add full Swift-DocC triple-slash documentation to the public
struct CmuxSidebarProviderRowAccessory, its public stored properties (kind,
systemImageName, defaultTab), the public initializer
(init(kind:systemImageName:defaultTab:)) and the public static inspector
constant: expand the brief summaries into a discussion paragraph describing
purpose and usage, document invariants/behavior, and add - Parameter callouts
for each initializer parameter explaining expected values and effects (e.g.,
what each CmuxSidebarProviderRowAccessoryKind value does, valid systemImageName
expectations, and defaultTab behavior); ensure the inspector constant has its
own summary/discussion noting it is a standard preconfigured accessory.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swift`:
- Around line 3-7: Add full Swift-DocC triple-slash documentation for the
exported enum CmuxSidebarProviderRowAccessoryKind and its public member(s):
document the enum's purpose, when and where clients should use it, any
invariants or expectations (e.g., that it is Codable/Equatable/Sendable and
backed by a String raw value), and describe serialization/compatibility
guarantees; for the case workspaceInspector add a clear sentence about what UI
affordance it represents and any behavior/constraints (for example whether it
always opens the workspace inspector, platform limits, or visibility rules).
Place the comments immediately above the enum declaration and above the case
using triple-slash (///) DocC style so the symbol and member are fully
documented for clients and documentation generation.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swift`:
- Around line 3-22: Add full Swift-DocC triple-slash documentation to the public
type CmuxSidebarProviderSection, each public property (id, treeSection, rows),
and the public initializer init(id:treeSection:rows:). For each symbol follow
the package guideline: start with a one-sentence summary on the first line, add
a blank /// line, then a short discussion paragraph describing purpose/behavior
and any invariants; for the initializer include a /// - Parameters: block
documenting id, treeSection, and rows. Ensure comments are triple-slash (///)
and placed immediately above the struct, each property, and the initializer so
DocC picks them up.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swift`:
- Around line 3-15: Add full Swift-DocC triple-slash documentation for the
public symbol CmuxSidebarProviderGitBranch: start with a one-sentence summary on
the first /// line, add a blank /// line, then a short discussion describing the
purpose and usage of this snapshot type; document the stored properties branch
and isDirty with brief descriptions; and add a documented initializer
init(branch:isDirty:) that includes /// - Parameters: with callouts for branch
and isDirty. Ensure every public symbol (struct, properties, and init) uses the
/// pattern and follows the package doc style.
In
`@Packages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swift`:
- Around line 25-28: Replace the hardcoded legacy keychain service/account
strings with the defined constants: use legacyKeychainService and
legacyKeychainAccount wherever the CFDictionary keychain queries are built (the
dictionaries currently containing "com.cmuxterm.app.socket-control" and
"local-socket-password"), including in the migration routine and
deleteLegacyPasswordFromKeychain(); update the query dictionaries to reference
these constants instead of string literals so the service/account names are
maintained in one place.
In `@Sources/ContentView.swift`:
- Around line 10356-10361: The view currently calls GhosttyConfig.load()
synchronously inside the SidebarTabItemSettingsStore initializer which may block
view init; change to construct SidebarTabItemSettingsStore with a fast default
(e.g. a default font size) and remove the synchronous GhosttyConfig.load() call,
then kick off an async load (using SidebarFontSizeProvider.loadFromGhosttyConfig
or a .task/async init helper) to fetch the real value and update the
SidebarTabItemSettingsStore instance when ready; target the
SidebarTabItemSettingsStore creation site and the async loader
SidebarFontSizeProvider.loadFromGhosttyConfig to implement the deferred load and
update logic.
---
Outside diff comments:
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`:
- Around line 37-42: The static constant cancelled on CmuxSidebarActionResult
currently embeds a hardcoded English message; replace the literal "Extension
action was cancelled" with a localized lookup using Bundle.module (e.g.,
NSLocalizedString or String(localized:bundle:)) so the user-facing text comes
from the package resource bundle, and add the corresponding key/value to the
package's Localizable.xcstrings resource; update
CmuxSidebarActionResult.cancelled to call the localization API with
Bundle.module and ensure the Localizable.xcstrings contains the translated
string for that key.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift`:
- Around line 101-104: Replace hardcoded English transport messages (e.g. the
"Waiting for cmux" message passed into deliver(.rejected(message), to: reply)
and other strings used when reporting connection state like in
report(.waitingForHost) or deliver(.rejected(...))) with localized strings from
your package bundle (e.g. use NSLocalizedString or a localized helper that loads
from Bundle.module) so these rejection messages can be localized or documented
as diagnostic-only; update the message creation in
CMUXSidebarExtensionConnection (the spots that call report(.waitingForHost,
ifCurrentGeneration: currentGeneration()) and deliver(.rejected(...), to:
reply)) to fetch a localized string or a clearly documented diagnostic message
provider.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swift`:
- Around line 37-38: The title/subtitle for the defaultWorkspaces provider
(created with CmuxSidebarProviderLocalizedText in
CmuxSidebarProviderDescriptor.swift using keys "sidebar.provider.default.title"
and "sidebar.provider.default.subtitle") are only localized for en/ja; add the
missing translations for all supported locales in
Resources/Localizable.xcstrings (e.g., fr, de, es, it, zh-Hans, zh-Hant, etc.)
so these keys exist in each locale bundle, or update the localization catalog to
include those keys for every supported locale to avoid English fallbacks.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift`:
- Around line 3-42: Add full Swift-DocC documentation to the public API: provide
a triple-slash doc comment for the CmuxSidebarProviderTreeSection type
describing its purpose and invariants, then add triple-slash comments for each
public property (id, title, titleText, subtitle, subtitleText, systemImageName,
projectRootPath, workspaceIds) explaining meaning, expected formats (e.g.,
stable id, SF Symbols name), nilability, and any invariants (e.g., workspaceIds
non-empty if representing a workspace). Also document the public init(...) with
a DocC discussion and `@param-style` callouts for each parameter describing
allowed values and behavior (including defaults for optional parameters) and
note thread-safety/Sendable conformance where relevant.
🪄 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: 4e072668-a9ca-4dc2-a69d-bbc9583bb3cd
📒 Files selected for processing (46)
Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swiftExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/Resources/Localizable.xcstringsExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidator.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIconShape.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxMutableSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderMutation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderPresentation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderWorkspacePopoverTab.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxContextualSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderContext.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderWorkspace.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderRelativeDateStyle.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderText.swiftPackages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swiftResources/Localizable.xcstringsSources/ContentView.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatascripts/write-sidebar-extension-point.sh
💤 Files with no reviewable changes (2)
- scripts/write-sidebar-extension-point.sh
- Resources/Localizable.xcstrings
🛑 Comments failed to post (11)
Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swift (2)
1-53: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Convert this new test to Swift Testing.
This is a new, non-UI test file using
XCTest. Per the test guidelines, new unit/integration tests must use Swift Testing (import Testing,@Suite,@Test,#expect).♻️ Proposed conversion to Swift Testing
-import CmuxSidebarProviderKit -@testable import CmuxExtensionSidebarExamples -import XCTest +import CmuxSidebarProviderKit +import Testing +@testable import CmuxExtensionSidebarExamples-final class SidebarProviderExistentialDispatchTests: XCTestCase { +@Suite struct SidebarProviderExistentialDispatchTests { /// Super Compact lists every workspace in a single section, so calling it /// through the existential must yield exactly the same rows as the concrete /// call — not the empty protocol-extension default. - func testSuperCompactRendersIdenticallyThroughExistential() { + `@Test` func superCompactRendersIdenticallyThroughExistential() { let snapshot = Self.snapshot(workspaceCount: 3) let concrete = SuperCompactSidebar() let existential: any CmuxSidebarProvider = concrete let concreteModel = concrete.render(snapshot: snapshot) let existentialModel = existential.render(snapshot: snapshot) - XCTAssertFalse(concreteModel.sections.isEmpty, "concrete render should produce a section") - XCTAssertEqual(concreteModel.sections.flatMap(\.rows).count, 3) - XCTAssertEqual( - existentialModel.sections, - concreteModel.sections, - "render(snapshot:) must dynamic-dispatch through the existential, not the empty default" - ) + `#expect`(!concreteModel.sections.isEmpty, "concrete render should produce a section") + `#expect`(concreteModel.sections.flatMap(\.rows).count == 3) + `#expect`( + existentialModel.sections == concreteModel.sections, + "render(snapshot:) must dynamic-dispatch through the existential, not the empty default" + ) }- func testWorkspaceListingProvidersRenderRowsThroughExistential() { + `@Test` func workspaceListingProvidersRenderRowsThroughExistential() { let snapshot = Self.snapshot(workspaceCount: 4) ... for provider in SidebarExamples.providers where listingProviderIDs.contains(provider.descriptor.id) { let model = provider.render(snapshot: snapshot) - XCTAssertFalse( - model.sections.flatMap(\.rows).isEmpty, - "Provider \(provider.descriptor.id) rendered no rows through the existential" - ) + `#expect`( + !model.sections.flatMap(\.rows).isEmpty, + "Provider \(provider.descriptor.id) rendered no rows through the existential" + ) } }As per coding guidelines: "do not write new tests with
import XCTestunless they are UI tests."🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 13-13: Classes should have an explicit deinit method
(required_deinit)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swift` around lines 1 - 53, Convert this XCTest-based file to Swift Testing: replace "import XCTest" with "import Testing", change the XCTestCase class SidebarProviderExistentialDispatchTests into a Swift Testing suite (e.g. annotate a struct or enum with `@Suite` and keep the same name), mark the two test methods testSuperCompactRendersIdenticallyThroughExistential and testWorkspaceListingProvidersRenderRowsThroughExistential with `@Test`, and replace XCTAssert* calls with the corresponding `#expect`(...) matchers (e.g. `#expect`(concreteModel.sections.isEmpty).toBeFalse(), `#expect`(concreteModel.sections.flatMap(\.rows).count).toEqual(3), and `#expect`(existentialModel.sections).toEqual(concreteModel.sections); similarly use `#expect`(...).not.toBeEmpty() or equivalent for the provider loop). Keep references to SuperCompactSidebar, SidebarExamples.providers, provider.descriptor.id, and CmuxSidebarProvider render(snapshot:) usage unchanged except for assertion syntax and suite/test annotations.
46-52:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against a vacuous pass.
If none of the
listingProviderIDsmatch an actualprovider.descriptor.id(e.g., an ID is renamed/typo'd), thewherefilter yields nothing, the loop body never runs, and the test passes without asserting anything. Assert that the expected number of providers was exercised.🛡️ Proposed fix
+ var matched = 0 for provider in SidebarExamples.providers where listingProviderIDs.contains(provider.descriptor.id) { + matched += 1 let model = provider.render(snapshot: snapshot) XCTAssertFalse( model.sections.flatMap(\.rows).isEmpty, "Provider \(provider.descriptor.id) rendered no rows through the existential" ) } + XCTAssertEqual(matched, listingProviderIDs.count, "expected every listing provider ID to be exercised")📝 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.var matched = 0 for provider in SidebarExamples.providers where listingProviderIDs.contains(provider.descriptor.id) { matched += 1 let model = provider.render(snapshot: snapshot) XCTAssertFalse( model.sections.flatMap(\.rows).isEmpty, "Provider \(provider.descriptor.id) rendered no rows through the existential" ) } XCTAssertEqual(matched, listingProviderIDs.count, "expected every listing provider ID to be exercised")🤖 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 `@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swift` around lines 46 - 52, The test currently loops "for provider in SidebarExamples.providers where listingProviderIDs.contains(provider.descriptor.id)" and can vacuously pass if no providers match; update the test to assert that at least one provider was exercised by either (a) computing the matchedProviders via SidebarExamples.providers.filter { listingProviderIDs.contains($0.descriptor.id) } and XCTAssertGreaterThan(matchedProviders.count, 0) before iterating, or (b) keep the loop but increment a counter for each iteration and XCTAssertGreaterThan(counter, 0) after the loop; use the same provider.descriptor.id and model.sections.flatMap(\\.rows) checks inside the iteration as-is.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swift (1)
3-10: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Complete the DocC for this public package API.
CmuxSidebarProviderCommandResultis a new public package type, butinit(ok:)still lacks the required- Parameter ok:callout and the docs do not spell out whatok == falsemeans for callers.📝 Proposed doc update
/// Result returned after CMUX handles a provider mutation. +/// +/// Use this value to distinguish between commands the host completed and +/// commands it rejected or could not apply. public struct CmuxSidebarProviderCommandResult: Codable, Equatable, Sendable { /// Whether CMUX accepted and completed the command. public var ok: Bool /// Creates a command result. + /// + /// - Parameter ok: `true` when the host accepted and completed the command; + /// otherwise `false`. public init(ok: Bool) { self.ok = ok } }As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "use- Parameter name:callouts".🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swift` around lines 3 - 10, The public type CmuxSidebarProviderCommandResult and its initializer init(ok:) lack complete DocC comments: update the triple-slash docs for the struct, the public var ok and the public init(ok:) to include a summary sentence for the type, a clear description of the ok property, and a "- Parameter ok:" callout that explains what true and false mean for callers (e.g., ok == true means CMUX accepted and completed the command; ok == false means the command was rejected or failed and callers should treat the result as unsuccessful). Ensure comments are placed immediately above CmuxSidebarProviderCommandResult, above the ok property, and above init(ok:) using Swift-DocC triple-slash style.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swift (1)
11-24:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
targetIndexcurrently allows impossible negative positions.Because this is a public
Codabletransport type,Intallows negative indices to be constructed and decoded even if you later add an initializer precondition. Please encode the non-negative invariant in the type itself, or add customDecodablevalidation that rejects negatives at the boundary.🧭 Safer direction
- public var targetIndex: Int + public var targetIndex: UIntIf you need to keep
Intfor downstream APIs, add a custominit(from:)and fail decoding for values< 0instead of relying on the memberwise initializer.As per coding guidelines, "Prefer compile-time invariants to runtime traps; encode 'programmer error' cases in the type system".
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swift` around lines 11 - 24, The public transport type allows negative targetIndex values which can be decoded or constructed; fix by enforcing a non-negative invariant at decode-time or via a refined type: add a custom Decodable init(from:) on CmuxSidebarProviderWorkspaceMove that reads targetIndex and throws a decoding error if value < 0 (or replace targetIndex with a NonNegativeInt wrapper type that validates on init/Decodable), and update the public init/workflow to accept/produce only the validated value so negatives cannot flow through the API.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift (1)
3-38: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Complete the required DocC for this public package API.
The type, its public properties, and
init(...)currently only have short summary comments. For newPackages/public APIs, this repo requires full Swift-DocC coverage, including the summary/discussion shape and- Parametercallouts on the initializer.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "use one-sentence summary on first line, blank///line, then discussion" plus- Parameter/- Returns:/- Throws:callouts where applicable.🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swift` around lines 3 - 38, Add full Swift-DocC comments for the public type CmuxSidebarProviderRow, its public stored properties (id, title, workspaceId, accessory, subtitle, trailingText, leadingIcon) and the public initializer; for each symbol use triple-slash comments with a one-sentence summary on the first line, a blank line, then a short discussion paragraph, and for the init include a detailed "- Parameter" entry for every parameter (and "- Returns:" only if applicable) following DocC style; update the existing short summaries above the struct and above init(...) to the required summary/discussion shape and add per-parameter callouts to the init signature so the package meets the Packages/ Swift-DocC documentation policy.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift (1)
3-29: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Fill in the package-required DocC for the new public symbols.
CmuxSidebarProviderRowAccessory, its public members, and the initializer are only documented with brief summaries right now. This package rule requires full DocC on newpublicAPIs, including discussion text and initializer parameter callouts.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "document enums/inits/properties/methods with meaning and invariants; include- Parameter name:callouts."🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swift` around lines 3 - 29, Add full Swift-DocC triple-slash documentation to the public struct CmuxSidebarProviderRowAccessory, its public stored properties (kind, systemImageName, defaultTab), the public initializer (init(kind:systemImageName:defaultTab:)) and the public static inspector constant: expand the brief summaries into a discussion paragraph describing purpose and usage, document invariants/behavior, and add - Parameter callouts for each initializer parameter explaining expected values and effects (e.g., what each CmuxSidebarProviderRowAccessoryKind value does, valid systemImageName expectations, and defaultTab behavior); ensure the inspector constant has its own summary/discussion noting it is a standard preconfigured accessory.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swift (1)
3-7: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add full DocC for this exported enum.
This is a new
publicpackage enum, but the docs stop at one-line summaries. The package guideline still requires full Swift-DocC structure for the type and public members.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "document enums/inits/properties/methods with meaning and invariants."🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swift` around lines 3 - 7, Add full Swift-DocC triple-slash documentation for the exported enum CmuxSidebarProviderRowAccessoryKind and its public member(s): document the enum's purpose, when and where clients should use it, any invariants or expectations (e.g., that it is Codable/Equatable/Sendable and backed by a String raw value), and describe serialization/compatibility guarantees; for the case workspaceInspector add a clear sentence about what UI affordance it represents and any behavior/constraints (for example whether it always opens the workspace inspector, platform limits, or visibility rules). Place the comments immediately above the enum declaration and above the case using triple-slash (///) DocC style so the symbol and member are fully documented for clients and documentation generation.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swift (1)
3-22: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Bring this public section model up to the package DocC standard.
The type, properties, and initializer only have terse summaries. New public SwiftPM APIs in
Packages/need full DocC coverage, including discussion text and initializer parameter documentation.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "use one-sentence summary on first line, blank///line, then discussion" with parameter callouts where applicable.🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swift` around lines 3 - 22, Add full Swift-DocC triple-slash documentation to the public type CmuxSidebarProviderSection, each public property (id, treeSection, rows), and the public initializer init(id:treeSection:rows:). For each symbol follow the package guideline: start with a one-sentence summary on the first line, add a blank /// line, then a short discussion paragraph describing purpose/behavior and any invariants; for the initializer include a /// - Parameters: block documenting id, treeSection, and rows. Ensure comments are triple-slash (///) and placed immediately above the struct, each property, and the initializer so DocC picks them up.Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swift (1)
3-15: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document this public snapshot type to the package standard.
The current comments are only short summaries. New public SwiftPM symbols in
Packages/need full DocC, andinit(branch:isDirty:)should include parameter callouts.As per coding guidelines, "Document every
publicsymbol in new Swift packages underPackages/with Swift-DocC triple-slash comments at time of writing" and "use///doc comments for public symbols with one-sentence summary on first line, blank///line, then discussion."🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swift` around lines 3 - 15, Add full Swift-DocC triple-slash documentation for the public symbol CmuxSidebarProviderGitBranch: start with a one-sentence summary on the first /// line, add a blank /// line, then a short discussion describing the purpose and usage of this snapshot type; document the stored properties branch and isDirty with brief descriptions; and add a documented initializer init(branch:isDirty:) that includes /// - Parameters: with callouts for branch and isDirty. Ensure every public symbol (struct, properties, and init) uses the /// pattern and follows the package doc style.Packages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swift (1)
25-28: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use the defined constants instead of hardcoded strings.
Lines 27-28 define
legacyKeychainServiceandlegacyKeychainAccountconstants, but these are not used. Lines 258-259 and 279-280 hardcode the same string literals in the keychain query dictionaries. This creates a maintenance risk—if the service or account name changes, it must be updated in multiple places.♻️ Proposed fix
private static func loadLegacyPasswordFromKeychain() -> String? { `#if` canImport(Security) let query: [CFString: Any] = [ kSecClass: kSecClassGenericPassword, - kSecAttrService: "com.cmuxterm.app.socket-control", - kSecAttrAccount: "local-socket-password", + kSecAttrService: legacyKeychainService, + kSecAttrAccount: legacyKeychainAccount, kSecReturnData: true, kSecMatchLimit: kSecMatchLimitOne, ]Apply the same change to
deleteLegacyPasswordFromKeychain()at lines 279-280.🤖 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/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swift` around lines 25 - 28, Replace the hardcoded legacy keychain service/account strings with the defined constants: use legacyKeychainService and legacyKeychainAccount wherever the CFDictionary keychain queries are built (the dictionaries currently containing "com.cmuxterm.app.socket-control" and "local-socket-password"), including in the migration routine and deleteLegacyPasswordFromKeychain(); update the query dictionaries to reference these constants instead of string literals so the service/account names are maintained in one place.Sources/ContentView.swift (1)
10356-10361: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Synchronous config load during
@StateObjectinitialization.
GhosttyConfig.load()is called synchronously when the view initializes. If this config load is I/O-bound or slow, it could delay view initialization. The async path viaSidebarFontSizeProvider.loadFromGhosttyConfighandles subsequent refreshes, but the initial value blocks here.If
GhosttyConfig.load()is cached or fast in practice, this is acceptable. Otherwise, consider initializing with a default and letting the async refresh populate the actual value.🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 10356-10356: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 10357-10357: SwiftUI state properties should be private
(private_swiftui_state)
🤖 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/ContentView.swift` around lines 10356 - 10361, The view currently calls GhosttyConfig.load() synchronously inside the SidebarTabItemSettingsStore initializer which may block view init; change to construct SidebarTabItemSettingsStore with a fast default (e.g. a default font size) and remove the synchronous GhosttyConfig.load() call, then kick off an async load (using SidebarFontSizeProvider.loadFromGhosttyConfig or a .task/async init helper) to fetch the real value and update the SidebarTabItemSettingsStore instance when ready; target the SidebarTabItemSettingsStore creation site and the async loader SidebarFontSizeProvider.loadFromGhosttyConfig to implement the deferred load and update logic.
…ganization # Conflicts: # Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionSidebarProvider.swift # Sources/ContentView.swift # cmux.xcodeproj/project.pbxproj
azooz2003-bit
force-pushed
the
feat-extension-sdk-organization
branch
from
June 2, 2026 15:05
8173919 to
fad4790
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/reload.sh (1)
787-791:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace wall-clock sleeps with state-based synchronization.
reload.shstill waits for build/quit/launch readiness via fixedsleepcalls and socket polling in Lines 787, 965-976, and 1064-1102. That makes tagged reloads flaky on slower machines and is explicitly disallowed for runtime/build scripts here. Please switch these paths to waiting on the actual condition with a bounded timeout instead of sleeping and hoping the process state caught up.As per coding guidelines "fail when the diff violates
.github/review-bot-rules/runtime-no-hacky-sleeps.md: fixed sleeps, delayed dispatch, timers, polling, or wall-clock waits used to paper over lifecycle, focus, rendering, socket, process, filesystem, network, teardown, startup, retry, or shared-state races".Also applies to: 965-976, 1064-1102
🤖 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 `@scripts/reload.sh` around lines 787 - 791, The current reload.sh uses fixed sleeps and ad-hoc socket polling (e.g., the "sleep 0.2" before checking $RELOAD_LOG and the later socket/poll blocks) which causes flaky timing; replace those sleeps/polls with explicit state-based waits with a bounded timeout: wait for the real condition (xcodebuild process exit/return code or presence of the completed build marker in $RELOAD_LOG, the target process PID becoming visible/terminated via ps/pgrep, or the launch socket/file descriptor becoming ready) using a loop that checks the concrete condition and breaks on success or after a configured timeout, and fail deterministically on timeout. Update the blocks that reference $RELOAD_LOG, the socket polling logic, and any retry loops so they use these condition checks and a clear timeout variable rather than sleep or fixed delays.Sources/ExtensionSidebarWorkspaceRowView.swift (1)
124-261: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider renaming internal types for consistency.
The internal types (
CmuxExtensionWorkspaceInspectorDraft,CmuxExtensionWorkspaceInspectorView,CmuxExtensionWorkspaceInspectorWindowContentView,CmuxExtensionWorkspaceInspectorBrowserView,CmuxExtensionSidebarInspectorWindowController) retain theCmuxExtensionprefix while they now work exclusively withCmuxSidebarProvider*types. For clarity, consider renaming them to useCmuxSidebarProviderorCmuxprefixes instead.🤖 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/ExtensionSidebarWorkspaceRowView.swift` around lines 124 - 261, The internal types use the outdated CmuxExtension prefix while they operate on CmuxSidebarProvider* models; rename CmuxExtensionWorkspaceInspectorDraft, CmuxExtensionWorkspaceInspectorView, CmuxExtensionWorkspaceInspectorWindowContentView, CmuxExtensionWorkspaceInspectorBrowserView, and CmuxExtensionSidebarInspectorWindowController to use a consistent CmuxSidebarProvider (or shorter Cmux) prefix (e.g., CmuxSidebarProviderWorkspaceInspectorDraft, CmuxSidebarProviderWorkspaceInspectorView, CmuxSidebarProviderWorkspaceInspectorWindowContentView, CmuxSidebarProviderWorkspaceInspectorBrowserView, CmuxSidebarProviderSidebarInspectorWindowController), and update all references/usages including the initial(...) factory, the State initialValue, initializer signatures, binding names ($draft), and any callers to these types so symbol names remain consistent across the module.Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swift (1)
3-69: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoffConvert this touched XCTest suite to Swift Testing.
This file is being modified, and the repo convention is to migrate touched XCTest tests in place:
XCTestCase→@Suitestruct,func testFoo()→@Test func foo(),XCTAssertEqual(a, b)→#expect(a == b),XCTAssertNil(x)→#expect(x == nil), andXCTUnwrap(x)→try#require(x).As per coding guidelines: "When touching existing XCTest tests, convert in place ...
XCTestCase→@Suite struct/final class;func testFoo()→@Test func foo();XCTAssertEqual(a, b)→#expect(a == b)...XCTUnwrap(x)→try#require(x)".🤖 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 `@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swift` around lines 3 - 69, Convert the XCTest-based suite to Swift Testing: change final class AttentionQueueSidebarTests: XCTestCase to `@Suite` struct AttentionQueueSidebarTests, convert each test method like func testLocalDisconnectedWorkspaceRemainsQuiet() throws to `@Test` func localDisconnectedWorkspaceRemainsQuiet() throws (drop the "test" prefix), replace XCTAssertNil(...) with `#expect`(... == nil), XCTAssertEqual(a, b) with `#expect`(a == b), and replace try XCTUnwrap(x) with try `#require`(x); keep the helper workspace(...) function and all usages of CmuxSidebarProviderSnapshot and AttentionQueueSidebar().render unchanged. Ensure imports and any test attributes required by Swift Testing are present elsewhere in the file/project.
♻️ Duplicate comments (1)
Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift (1)
23-32: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winInconsistent optional-parameter defaults.
titleTextandsubtitleTextdefault tonil, but the equally optionalsubtitleandprojectRootPathrequire explicit arguments. Default them tonilfor consistency and ergonomic call sites.♻️ Proposed fix
public init( id: String, title: String, titleText: CmuxSidebarProviderLocalizedText? = nil, - subtitle: String?, + subtitle: String? = nil, subtitleText: CmuxSidebarProviderLocalizedText? = nil, systemImageName: String, - projectRootPath: String?, + projectRootPath: String? = nil, workspaceIds: [UUID] ) {🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift` around lines 23 - 32, The initializer for CmuxSidebarProviderTreeSection has inconsistent optional-parameter defaults; make the optional parameters subtitle and projectRootPath default to nil like titleText and subtitleText by updating the CmuxSidebarProviderTreeSection.init signature to use subtitle: String? = nil and projectRootPath: String? = nil, and then update any call sites that relied on the previous required arguments to omit those parameters where appropriate.
🤖 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 `@cmux.xcodeproj/project.pbxproj`:
- Around line 1945-1947: cmuxTests is missing the new CmuxSidebarProviderKit
package wiring so test builds will fail when SidebarProviderMenuRegressionTests
imports it; add an XCLocalSwiftPackageReference for the provider and an
XCSwiftPackageProductDependency referencing "CmuxSidebarProviderKit" for the
cmuxTests target, create the corresponding PBXBuildFile entry and include it in
the cmuxTests Frameworks build phase (mirror the entries you added for cmux and
cmux-unit), and ensure the package product dependency is also added to the
cmux-unit/ cmux targets as per the existing pattern used for
CmuxExtensionHostSupport/CmuxExtensionKit.
In
`@Examples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarExtension.swift`:
- Around line 72-81: The apply(_ operation: ) helper currently treats all errors
the same; add an explicit handler for the cancellation case so stale errorText
is cleared instead of showing the generic denial message. Update the apply
function to catch CmuxSidebarActionError.cancelled (or catch CancellationError
if your code uses standard cancellation) before the generic catch, and set
errorText = nil in that branch; keep the existing catch
CmuxSidebarActionError.rejected(let message) to set the rejection message and
the final generic catch to set the localized "tabsVisible.actionDenied" text.
In `@Packages/CMUXExtensionHostSupport/Package.swift`:
- Around line 18-25: Add a test target to the package and scaffold unit tests
for the host view and browser presenter: update the Package.swift to include a
.testTarget referencing "CMUXExtensionHostSupport" as a dependency, and create a
Tests directory with test files that exercise CMUXSidebarExtensionHostView and
CMUXSidebarExtensionBrowserPresenter behaviors (initialization, state
transitions, and key public methods); ensure tests import CmuxExtensionKit where
needed and cover edge cases for presenter logic and view lifecycle.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift`:
- Line 47: The decoder currently requires readScopes by calling readScopes = try
container.decode([CmuxExtensionScope].self, forKey: .readScopes) in init(from:),
but the public initializer defaults readScopes to [] which makes the key
effectively optional; update init(from:) to use decodeIfPresent for readScopes
and fall back to [] (e.g., readScopes = try
container.decodeIfPresent([CmuxExtensionScope].self, forKey: .readScopes) ?? [])
to match actionScopes and the public init behavior, ensuring round-trip Codable
compatibility for CmuxExtensionManifest.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`:
- Around line 1-5: The file name does not match its public API type: rename the
source file currently named CMUXExtensionActionResult.swift to
CmuxSidebarActionResult.swift and update any imports/targets referring to the
old filename; ensure the primary public type CmuxSidebarActionResult (and
related enums like the rejection-reason and error enums defined in the same
file) remain in this single file so each public API type lives in its own file
named after the type.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift`:
- Line 20: The Info.plist key used by CmuxSidebarExtensionPoint no longer
matches the constant identifierInfoPlistKey
("CmuxSidebarExtensionPointIdentifier"), causing identifier resolution in
CmuxSidebarExtensionPoint.identifier() to miss the override and fall back to
baseIdentifier; fix this by updating Resources/Info.plist to rename the existing
CMUXSidebarExtensionPointIdentifier entry to CmuxSidebarExtensionPointIdentifier
(or revert the constant if you intend to keep the old key) so the
identifierInfoPlistKey lookup in CmuxSidebarExtensionPoint.identifier() reads
the intended override.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swift`:
- Around line 3-5: The documentation comment is placed between the attribute and
the type declaration; move the doc comment so it appears immediately before the
`@_spi`(CmuxHostTransport) attribute (i.e., place the doc comment above the
attribute) so tooling sees the doc comment associated with the
CmuxSidebarHostClient struct; update the placement around the
`@_spi`(CmuxHostTransport) and public struct CmuxSidebarHostClient: Sendable
declaration accordingly.
In
`@Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swift`:
- Around line 74-82: Add a short clarifying comment near the lossy decoding util
and a note in CMUXSidebarSnapshot explaining the fail-closed, forward-compat
behavior: reference
KeyedDecodingContainer.decodeLossySetIfPresent(Value.Type,forKey:) to state that
unknown scope raw values are dropped (treated as not granted), and mention that
host-side enforcement (in CMUXInstalledExtensionSidebarHostView) will reject
actions unless allowedActionScopes is a superset of action.requiredScopes; also
document the workspaceMetadata-missing behavior in
CMUXSidebarSnapshot.filtered(for:actionScopes:) that causes workspace.title to
be set to an empty string.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxMutableSidebarProvider.swift`:
- Around line 5-9: Update the public protocol requirement declaration for
handle(_:snapshot:) to include DocC callouts: add a one-line summary plus "-
Parameter mutation: CmuxSidebarProviderMutation — description of the mutation
input", "- Parameter snapshot: CmuxSidebarProviderSnapshot — description of the
snapshot input", "- Returns: CmuxSidebarProviderCommandResult — describe what
the result represents", and "- Throws: — describe under what conditions the
method throws"; target the protocol method named handle(_:snapshot:) in
CmuxMutableSidebarProvider and reference the types CmuxSidebarProviderMutation,
CmuxSidebarProviderSnapshot, and CmuxSidebarProviderCommandResult so the
documentation appears for this public symbol.
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider`+Rendering.swift:
- Around line 13-19: The extension-provided render(snapshot:context:) is
currently dropping the context by forwarding to render(snapshot:), so ensure
contextual rendering isn’t lost: change the implementation of
render(snapshot:context:) on CmuxSidebarProvider to detect and call the
contextual implementation when available (i.e., if self conforms to
CmuxContextualSidebarProvider, invoke that provider’s render(snapshot:context:)
using the provided context), otherwise fall back to calling render(snapshot:).
Update the extension containing render(snapshot:context:) (and keep names
render(snapshot:) and render(snapshot:context:) unchanged) so context.now and
other contextual data are preserved for contextual providers.
---
Outside diff comments:
In
`@Examples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swift`:
- Around line 3-69: Convert the XCTest-based suite to Swift Testing: change
final class AttentionQueueSidebarTests: XCTestCase to `@Suite` struct
AttentionQueueSidebarTests, convert each test method like func
testLocalDisconnectedWorkspaceRemainsQuiet() throws to `@Test` func
localDisconnectedWorkspaceRemainsQuiet() throws (drop the "test" prefix),
replace XCTAssertNil(...) with `#expect`(... == nil), XCTAssertEqual(a, b) with
`#expect`(a == b), and replace try XCTUnwrap(x) with try `#require`(x); keep the
helper workspace(...) function and all usages of CmuxSidebarProviderSnapshot and
AttentionQueueSidebar().render unchanged. Ensure imports and any test attributes
required by Swift Testing are present elsewhere in the file/project.
In `@scripts/reload.sh`:
- Around line 787-791: The current reload.sh uses fixed sleeps and ad-hoc socket
polling (e.g., the "sleep 0.2" before checking $RELOAD_LOG and the later
socket/poll blocks) which causes flaky timing; replace those sleeps/polls with
explicit state-based waits with a bounded timeout: wait for the real condition
(xcodebuild process exit/return code or presence of the completed build marker
in $RELOAD_LOG, the target process PID becoming visible/terminated via ps/pgrep,
or the launch socket/file descriptor becoming ready) using a loop that checks
the concrete condition and breaks on success or after a configured timeout, and
fail deterministically on timeout. Update the blocks that reference $RELOAD_LOG,
the socket polling logic, and any retry loops so they use these condition checks
and a clear timeout variable rather than sleep or fixed delays.
In `@Sources/ExtensionSidebarWorkspaceRowView.swift`:
- Around line 124-261: The internal types use the outdated CmuxExtension prefix
while they operate on CmuxSidebarProvider* models; rename
CmuxExtensionWorkspaceInspectorDraft, CmuxExtensionWorkspaceInspectorView,
CmuxExtensionWorkspaceInspectorWindowContentView,
CmuxExtensionWorkspaceInspectorBrowserView, and
CmuxExtensionSidebarInspectorWindowController to use a consistent
CmuxSidebarProvider (or shorter Cmux) prefix (e.g.,
CmuxSidebarProviderWorkspaceInspectorDraft,
CmuxSidebarProviderWorkspaceInspectorView,
CmuxSidebarProviderWorkspaceInspectorWindowContentView,
CmuxSidebarProviderWorkspaceInspectorBrowserView,
CmuxSidebarProviderSidebarInspectorWindowController), and update all
references/usages including the initial(...) factory, the State initialValue,
initializer signatures, binding names ($draft), and any callers to these types
so symbol names remain consistent across the module.
---
Duplicate comments:
In
`@Packages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swift`:
- Around line 23-32: The initializer for CmuxSidebarProviderTreeSection has
inconsistent optional-parameter defaults; make the optional parameters subtitle
and projectRootPath default to nil like titleText and subtitleText by updating
the CmuxSidebarProviderTreeSection.init signature to use subtitle: String? = nil
and projectRootPath: String? = nil, and then update any call sites that relied
on the previous required arguments to omit those parameters where appropriate.
🪄 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: fd91e3ae-4df0-4398-a5f7-d620421d816a
📒 Files selected for processing (108)
Examples/CmuxExtensionSidebarExamples/Package.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/AttentionQueueSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/BrowserStackSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/DevServerSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/LastPromptSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/ProjectWorktreeSidebar.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SidebarExamples.swiftExamples/CmuxExtensionSidebarExamples/Sources/CmuxExtensionSidebarExamples/SuperCompactSidebar.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/AttentionQueueSidebarTests.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/BrowserStackSidebarTests.swiftExamples/CmuxExtensionSidebarExamples/Tests/CmuxExtensionSidebarExamplesTests/SidebarProviderExistentialDispatchTests.swiftExamples/SampleSidebarExtensionApp/README.mdExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Connection/SidebarConnectionModel.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Extension/SampleSidebarExtension.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Localizable.xcstringsExamples/SampleSidebarExtensionApp/SampleSidebarExtension/Model/SidebarInsightModel.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtension/UI/SampleSidebarView.swiftExamples/SampleSidebarExtensionApp/SampleSidebarExtensionApp/Localizable.xcstringsExamples/StubAgentSidebarExtension/Package.swiftExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/Resources/Localizable.xcstringsExamples/StubAgentSidebarExtension/Sources/StubAgentSidebarExtension/StubAgentSidebarExtension.swiftExamples/TabsVisibleSidebar/TabsVisibleSidebar.xcodeproj/project.pbxprojExamples/TabsVisibleSidebar/TabsVisibleSidebar/Localizable.xcstringsExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/Localizable.xcstringsExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarExtension.swiftExamples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarView.swiftPackages/CMUXExtensionClient/README.mdPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXInstalledSidebarExtension.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXSidebarExtensionDiscovery.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXExtensionClientError.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRecord.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRegistry.swiftPackages/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swiftPackages/CMUXExtensionClient/Tests/CMUXExtensionClientTests/CMUXExtensionClientTests.swiftPackages/CMUXExtensionHostSupport/Package.swiftPackages/CMUXExtensionHostSupport/README.mdPackages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/Browser/CMUXSidebarExtensionBrowserPresenter.swiftPackages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/Hosting/CMUXSidebarExtensionHostView.swiftPackages/CmuxExtensionKit/README.mdPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/CmuxExtensionCompatibility.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Common/CMUXExtensionAPIVersion.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxHost.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxUIExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionLocalizedText.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionSidebarProvider.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionSidebarProviderDescriptor.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionKind.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionScope.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarAction.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtension.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSurface.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarWorkspace.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarXPC.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarActionCancellation.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarContext.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionRuntime.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarExtensionScene.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarHost.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidationError.swiftPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidator.swiftPackages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swiftPackages/CmuxSidebarProviderKit/Package.swiftPackages/CmuxSidebarProviderKit/README.mdPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderDescriptor.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/CmuxSidebarProviderLocalizedText.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIcon.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Icons/CmuxSidebarProviderIconShape.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxMutableSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderCommandResult.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderMutation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderPresentationRequest.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxSidebarProviderWorkspaceMove.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderPresentation.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Presentation/CmuxSidebarProviderWorkspacePopoverTab.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxContextualSidebarProvider.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProvider+Rendering.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderContext.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rendering/CmuxSidebarProviderRenderModel.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRow.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessory.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Rows/CmuxSidebarProviderRowAccessoryKind.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Sections/CmuxSidebarProviderTreeSection.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderGitBranch.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderSnapshot.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Snapshot/CmuxSidebarProviderWorkspace.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderRelativeDateStyle.swiftPackages/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Text/CmuxSidebarProviderText.swiftPackages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swiftResources/Localizable.xcstringsResources/com.manaflow.cmux.sidebar.appextensionpointSources/CMUXInstalledExtensionSidebarHostView.swiftSources/CMUXSidebarExtensionBrowserPanel.swiftSources/ContentView.swiftSources/ExtensionSidebarWorkspaceRowView.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatacmuxTests/SidebarProviderMenuRegressionTests.swiftscripts/reload.shscripts/write-sidebar-extension-point.sh
💤 Files with no reviewable changes (18)
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRecord.swift
- Packages/CMUXExtensionClient/Tests/CMUXExtensionClientTests/CMUXExtensionClientTests.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXInstalledSidebarExtension.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionSidebarProviderDescriptor.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionKind.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionLocalizedText.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxHost.swift
- Resources/com.manaflow.cmux.sidebar.appextensionpoint
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Discovery/CMUXSidebarExtensionDiscovery.swift
- Packages/CMUXExtensionClient/README.md
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxUIExtension.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/HostCompatibility/CmuxExtensionSidebarProvider.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Session/CMUXSidebarExtensionSession.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXExtensionClientError.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/CmuxExtensionCompatibility.swift
- Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Extension/CmuxExtension.swift
- Packages/CMUXExtensionClient/Sources/CMUXExtensionClient/Registry/CMUXSidebarExtensionRegistry.swift
- Packages/CmuxSocketControl/Sources/CmuxSocketControl/SocketControlPasswordStore.swift
Comment on lines
+72
to
81
| private func apply(_ operation: () async throws -> Void) async { | ||
| do { | ||
| try await operation() | ||
| errorText = nil | ||
| } else { | ||
| errorText = result.message ?? String(localized: "tabsVisible.actionDenied", defaultValue: "cmux did not allow that action") | ||
| } catch CmuxSidebarActionError.rejected(let message) { | ||
| errorText = message | ||
| } catch { | ||
| errorText = String(localized: "tabsVisible.actionDenied", defaultValue: "cmux did not allow that action") | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle .cancelled explicitly to avoid showing stale error text.
The apply helper currently treats cancellation the same as other errors, setting errorText to "cmux did not allow that action". Cancellation is not an error condition and should clear any existing error text instead. The StubAgentSidebarExtension example handles this correctly by clearing errorText on .cancelled.
🔧 Proposed fix
private func apply(_ operation: () async throws -> Void) async {
do {
try await operation()
errorText = nil
} catch CmuxSidebarActionError.rejected(let message) {
errorText = message
+ } catch CmuxSidebarActionError.cancelled {
+ errorText = nil
} catch {
errorText = String(localized: "tabsVisible.actionDenied", defaultValue: "cmux did not allow that action")
}
}🤖 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
`@Examples/TabsVisibleSidebar/TabsVisibleSidebarExtension/TabsVisibleSidebarExtension.swift`
around lines 72 - 81, The apply(_ operation: ) helper currently treats all
errors the same; add an explicit handler for the cancellation case so stale
errorText is cleared instead of showing the generic denial message. Update the
apply function to catch CmuxSidebarActionError.cancelled (or catch
CancellationError if your code uses standard cancellation) before the generic
catch, and set errorText = nil in that branch; keep the existing catch
CmuxSidebarActionError.rejected(let message) to set the rejection message and
the final generic catch to set the localized "tabsVisible.actionDenied" text.
Comment on lines
18
to
25
| targets: [ | ||
| .target( | ||
| name: "CMUXExtensionClient", | ||
| name: "CMUXExtensionHostSupport", | ||
| dependencies: [ | ||
| .product(name: "CmuxExtensionKit", package: "CmuxExtensionKit"), | ||
| ] | ||
| ), | ||
| .testTarget( | ||
| name: "CMUXExtensionClientTests", | ||
| dependencies: ["CMUXExtensionClient"] | ||
| ), | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check that the source directory exists and has Swift files
fd -t f '\.swift$' 'Packages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport' --exec echo "Found: {}"
# Check if any test files exist under CMUXExtensionHostSupport
fd -t f '\.swift$' 'Packages/CMUXExtensionHostSupport/Tests' --exec echo "Test file: {}" 2>/dev/null || echo "No test directory found"Repository: manaflow-ai/cmux
Length of output: 326
Add/replace test coverage for CMUXExtensionHostSupport
Packages/CMUXExtensionHostSupport/Package.swiftdefines only a single.target(CMUXExtensionHostSupport) and no.testTarget.- The sources exist under
Packages/CMUXExtensionHostSupport/Sources/CMUXExtensionHostSupport/(e.g.,Hosting/CMUXSidebarExtensionHostView.swift,Browser/CMUXSidebarExtensionBrowserPresenter.swift), but there is noPackages/CMUXExtensionHostSupport/Testsdirectory. - Consider adding replacement unit tests for the host view and browser presenter logic.
🤖 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/CMUXExtensionHostSupport/Package.swift` around lines 18 - 25, Add a
test target to the package and scaffold unit tests for the host view and browser
presenter: update the Package.swift to include a .testTarget referencing
"CMUXExtensionHostSupport" as a dependency, and create a Tests directory with
test files that exercise CMUXSidebarExtensionHostView and
CMUXSidebarExtensionBrowserPresenter behaviors (initialization, state
transitions, and key public methods); ensure tests import CmuxExtensionKit where
needed and cover edge cases for presenter logic and view lifecycle.
| [CMUXExtensionActionScope].self, | ||
| forKey: .requestedActionScopes | ||
| minimumAPIVersion = try container.decodeIfPresent(CmuxExtensionAPIVersion.self, forKey: .minimumAPIVersion) ?? .sidebarV2 | ||
| readScopes = try container.decode([CmuxExtensionScope].self, forKey: .readScopes) |
There was a problem hiding this comment.
Decoding requires readScopes but init defaults it to [].
The initializer defaults readScopes to [], but init(from:) uses decode (not decodeIfPresent), making readScopes a required key when decoding. This asymmetry will cause decode failures for payloads created with the default initializer if they're round-tripped through Codable without explicitly including readScopes.
Consider using decodeIfPresent with a ?? [] fallback for consistency with actionScopes and the public initializer:
🔧 Proposed fix
- readScopes = try container.decode([CmuxExtensionScope].self, forKey: .readScopes)
+ readScopes = try container.decodeIfPresent([CmuxExtensionScope].self, forKey: .readScopes) ?? []📝 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.
Suggested change
| readScopes = try container.decode([CmuxExtensionScope].self, forKey: .readScopes) | |
| readScopes = try container.decodeIfPresent([CmuxExtensionScope].self, forKey: .readScopes) ?? [] |
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionManifest.swift`
at line 47, The decoder currently requires readScopes by calling readScopes =
try container.decode([CmuxExtensionScope].self, forKey: .readScopes) in
init(from:), but the public initializer defaults readScopes to [] which makes
the key effectively optional; update init(from:) to use decodeIfPresent for
readScopes and fall back to [] (e.g., readScopes = try
container.decodeIfPresent([CmuxExtensionScope].self, forKey: .readScopes) ?? [])
to match actionScopes and the public init behavior, ensuring round-trip Codable
compatibility for CmuxExtensionManifest.
Comment on lines
1
to
+5
| import Foundation | ||
|
|
||
| public struct CMUXExtensionActionResult: Codable, Equatable, Sendable { | ||
| @_spi(CmuxHostTransport) | ||
| /// Result returned by CMUX for a sidebar host action request. | ||
| public struct CmuxSidebarActionResult: Codable, Equatable, Sendable { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Rename the file to match its primary type.
The file is still named CMUXExtensionActionResult.swift but now defines CmuxSidebarActionResult (plus the rejection-reason and error enums). Rename to CmuxSidebarActionResult.swift so the file matches the type it declares.
As per coding guidelines: "each struct, class, enum ... that is part of a public API ... lives in its own file named after the type."
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXExtensionActionResult.swift`
around lines 1 - 5, The file name does not match its public API type: rename the
source file currently named CMUXExtensionActionResult.swift to
CmuxSidebarActionResult.swift and update any imports/targets referring to the
old filename; ensure the primary public type CmuxSidebarActionResult (and
related enums like the rejection-reason and error enums defined in the same
file) remain in this single file so each public API type lives in its own file
named after the type.
| /// (via Info.plist variable substitution), so the resolved id lives only in the built | ||
| /// bundle, never in tracked source. Absent or empty means "use ``baseIdentifier``". | ||
| public static let identifierInfoPlistKey = "CMUXSidebarExtensionPointIdentifier" | ||
| public static let identifierInfoPlistKey = "CmuxSidebarExtensionPointIdentifier" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if extension point generation scripts reference the new key name
rg -n 'CmuxSidebarExtensionPointIdentifier|CMUXSidebarExtensionPointIdentifier' --type sh --type swift -g '!*.swift.gyb'Repository: manaflow-ai/cmux
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any remaining references to the old/new key names across the repo
rg -n 'CMUXSidebarExtensionPointIdentifier|CmuxSidebarExtensionPointIdentifier' --hidden --glob '!.git/*'
echo "---- usage of identifierInfoPlistKey symbol ----"
rg -n 'identifierInfoPlistKey' --hidden --glob '!.git/*'
echo "---- potential Info.plist writers / extension point generation ----"
# Look for files that mention "extension point" and/or the old/new key
rg -n 'extension point|Info\.plist|identifierInfoPlistKey|SidebarExtensionPoint' --hidden --glob '!.git/*' --glob '*.{sh,swift,py,js,ts,mjs,cjs,yml,yaml,json,tml,txt,rb,go,Makefile}'Repository: manaflow-ai/cmux
Length of output: 14026
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the Swift implementation around identifierInfoPlistKey + fallback behavior
sed -n '1,120p' Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift
# 2) Inspect how the base app Info.plist is declared (does it still use the old key?)
sed -n '1,120p' Resources/Info.plist
# 3) Inspect build-time generation script that writes extension point declaration
sed -n '1,120p' scripts/write-sidebar-extension-point.sh
# 4) Inspect reload-extension script notes about baked EXExtensionPointIdentifier
sed -n '1,220p' scripts/reload-extension.sh
# 5) Search scripts for both key names (broader than just sh/swift)
rg -n 'CMUXSidebarExtensionPointIdentifier|CmuxSidebarExtensionPointIdentifier' scripts Resources Packages --hidden --glob '!.git/*'Repository: manaflow-ai/cmux
Length of output: 16027
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for both key spellings across Info.plist templates/resources and example projects.
rg -n 'CMUXSidebarExtensionPointIdentifier|CmuxSidebarExtensionPointIdentifier' \
--hidden --glob '!.git/*' \
--glob '*/Info.plist' --glob '*/Info.plist.*' --glob '*.plist' --glob '*.xib' --glob '*.storyboard' \
Examples Packages Resources .github scripts --glob '!*swift.gyb' --glob '!*swift.gyb/*' || true
# Find where the host reads the extension point identifier at runtime.
rg -n 'CmuxSidebarExtensionPoint\.identifier(\(|\b)' --hidden --glob '!.git/*' Packages || true
# Inspect host view around the call site.
FILE='Sources/CMUXInstalledExtensionSidebarHostView.swift'
if [ -f "$FILE" ]; then
sed -n '240,340p' "$FILE"
fiRepository: manaflow-ai/cmux
Length of output: 3764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'CmuxSidebarExtensionPoint\.identifier\s*\(' --hidden --glob '!.git/*' .
rg -n 'identifierInfoPlistKey|CMUXSidebarExtensionPointIdentifier|CmuxSidebarExtensionPointIdentifier' --hidden --glob '!.git/*' --glob '*.plist' --glob '*.pbxproj' --glob '*.*' \
Examples Packages Resources scripts .github
# Focus on example extension projects' Info.plist templates
fd -t f -e plist -e plist.in -e xcconfig -e xib -e storyboard Examples Packages Resources scripts .github | head -n 50Repository: manaflow-ai/cmux
Length of output: 1150
Update Info.plist key to match identifierInfoPlistKey (breaks sidebar extension point resolution)
CmuxSidebarExtensionPoint.identifier()now readsidentifierInfoPlistKey = "CmuxSidebarExtensionPointIdentifier"and falls back tobaseIdentifierwhen the key is absent/empty.Resources/Info.pliststill declares the old keyCMUXSidebarExtensionPointIdentifier(set to$(CMUX_SIDEBAR_EXTENSION_POINT_ID)), so the override is never read and tag-scoped point IDs collapse tocom.manaflow.cmux.sidebar, leading to extension point mismatches (and old third-party bundles using the old key won’t override either).- Rename the key in
Resources/Info.plisttoCmuxSidebarExtensionPointIdentifierto match the new constant (or revert the constant if the old key must remain supported).
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionPoint.swift`
at line 20, The Info.plist key used by CmuxSidebarExtensionPoint no longer
matches the constant identifierInfoPlistKey
("CmuxSidebarExtensionPointIdentifier"), causing identifier resolution in
CmuxSidebarExtensionPoint.identifier() to miss the override and fall back to
baseIdentifier; fix this by updating Resources/Info.plist to rename the existing
CMUXSidebarExtensionPointIdentifier entry to CmuxSidebarExtensionPointIdentifier
(or revert the constant if you intend to keep the old key) so the
identifierInfoPlistKey lookup in CmuxSidebarExtensionPoint.identifier() reads
the intended override.
Comment on lines
+3
to
+5
| @_spi(CmuxHostTransport) | ||
| /// Host-side callbacks used by the sidebar XPC bridge. | ||
| public struct CmuxSidebarHostClient: Sendable { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Doc comment placement after @_spi attribute.
The doc comment on line 4 appears between the @_spi attribute and the struct declaration. While syntactically valid, the conventional placement is before all attributes for better tooling support.
📝 Suggested reordering
+/// Host-side callbacks used by the sidebar XPC bridge.
`@_spi`(CmuxHostTransport)
-/// Host-side callbacks used by the sidebar XPC bridge.
public struct CmuxSidebarHostClient: Sendable {📝 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.
Suggested change
| @_spi(CmuxHostTransport) | |
| /// Host-side callbacks used by the sidebar XPC bridge. | |
| public struct CmuxSidebarHostClient: Sendable { | |
| /// Host-side callbacks used by the sidebar XPC bridge. | |
| `@_spi`(CmuxHostTransport) | |
| public struct CmuxSidebarHostClient: Sendable { |
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarHostClient.swift`
around lines 3 - 5, The documentation comment is placed between the attribute
and the type declaration; move the doc comment so it appears immediately before
the `@_spi`(CmuxHostTransport) attribute (i.e., place the doc comment above the
attribute) so tooling sees the doc comment associated with the
CmuxSidebarHostClient struct; update the placement around the
`@_spi`(CmuxHostTransport) and public struct CmuxSidebarHostClient: Sendable
declaration accordingly.
Comment on lines
+74
to
+82
| private extension KeyedDecodingContainer { | ||
| func decodeLossySetIfPresent<Value>( | ||
| _ type: Value.Type, | ||
| forKey key: Key | ||
| ) throws -> Set<Value> where Value: RawRepresentable, Value.RawValue == String, Value: Hashable { | ||
| guard let rawValues = try decodeIfPresent([String].self, forKey: key) else { return [] } | ||
| return Set(rawValues.compactMap(type.init(rawValue:))) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for documentation of the lossy decoding contract and scope versioning strategy
rg -n -C3 --type=swift --type=md 'lossy.*decod|forward.compa|scope.*version|unknown.*scope' Packages/CmuxExtensionKit/Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swift"
echo "=== File header/context ==="
# Show first ~120 lines with numbers (avoid huge output)
sed -n '1,140p' "$FILE" | nl -ba
echo "=== Search for decodeLossySetIfPresent usage in repo ==="
rg -n "decodeLossySetIfPresent" .
echo "=== Search for sidebarV1/sidebarV2/apiVersion usage ==="
rg -n "sidebarV1|sidebarV2|apiVersion" Packages/CmuxExtensionKit/Sources
echo "=== Search for scope versioning / grant / permission terms in Swift/MD ==="
rg -n --type=swift --type=md 'scope.*version|lossy|forward compat|unknown.*scope|grant' Packages/CmuxExtensionKit/ .Repository: manaflow-ai/cmux
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swift"
echo "=== File context (first 200 lines, numbered) ==="
sed -n '1,200p' "$FILE" | cat -n
echo "=== decodeLossySetIfPresent usage in repo ==="
rg -n --hidden --glob '!**/.build/**' "decodeLossySetIfPresent" .
echo "=== sidebarV1/sidebarV2/apiVersion usage ==="
rg -n --hidden --glob '!**/.build/**' "(sidebarV1|sidebarV2|apiVersion)" Packages/CmuxExtensionKit/Sources
echo "=== docs/contracts mentioning scope versioning / lossy decoding ==="
rg -n --hidden --glob '!**/.build/**' --type=md '(lossy|forward compat|forward-compatible|scope.*version|unknown.*scope|grant)' .Repository: manaflow-ai/cmux
Length of output: 8615
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Packages/CmuxExtensionKit/README.md: around snapshot contract ==="
# Show around the previously hit line ~113
sed -n '80,170p' Packages/CmuxExtensionKit/README.md | cat -n
echo "=== CMUXExtensionHostSupport README: look for permission/grants/snapshot policy ==="
sed -n '1,220p' Packages/CMUXExtensionHostSupport/README.md | cat -n
echo "=== Search for scope/grant/snapshot filtering docs in CmuxExtensionKit package ==="
rg -n "grantedReadScopes|grantedActionScopes|CmuxExtensionScope|CmuxExtensionActionScope|workspaceMetadata|snapshot" Packages/CmuxExtensionKit/README.md Packages/CmuxExtensionKit/Sources Packages/CMUXExtensionHostSupport/README.md
echo "=== Locate CmuxExtensionScope / CmuxExtensionActionScope definitions ==="
rg -n "enum CmuxExtensionScope|struct CmuxExtensionScope|typealias CmuxExtensionScope|enum CmuxExtensionActionScope|typealias CmuxExtensionActionScope" Packages/CmuxExtensionKit/Sources
echo "=== Show CmuxExtensionScope / CmuxExtensionActionScope source files (line-limited) ==="
# Print matching files with some surrounding context
rg -n "enum CmuxExtensionScope|enum CmuxExtensionActionScope" --files-with-matches Packages/CmuxExtensionKit/Sources | sed -n '1,20p'Repository: manaflow-ai/cmux
Length of output: 16273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CMUXExtensionScope.swift ==="
sed -n '1,200p' Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Manifest/CMUXExtensionScope.swift | cat -n
echo "=== CMUXSidebarContext.swift docs ==="
sed -n '1,120p' Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CmuxSidebarContext.swift | cat -n
echo "=== Search where grantedReadScopes/grantedActionScopes are used ==="
rg -n "grantedReadScopes|grantedActionScopes" Packages/CmuxExtensionKit/Sources | head -n 200
echo "=== Search for forward-compat/lossy/unknown scope wording in README/docs ==="
rg -n --type=swift --type=md '(lossy|forward|compat|unknown.*scope|ignore.*unknown)' Packages/CmuxExtensionKit/README.md Packages/CmuxExtensionKit/Sources Packages/CMUXExtensionHostSupport/README.md | head -n 200Repository: manaflow-ai/cmux
Length of output: 5334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CMUXSidebarExtensionConnection.swift (first 260 lines) ==="
sed -n '1,260p' Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarExtensionConnection.swift | cat -n
echo "=== CMUXExtensionValidator.swift ==="
sed -n '1,220p' Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Validation/CMUXExtensionValidator.swift | cat -n
echo "=== Search Sources for compatibility/lossy/unknown/forward wording ==="
rg -n --hidden --glob '!**/.build/**' '(compatib|lossy|forward|unknown.*scope)' Packages/CmuxExtensionKit/Sources | head -n 200Repository: manaflow-ai/cmux
Length of output: 14261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== grantedActionScopes usage across repo ==="
rg -n --hidden --glob '!**/.build/**' "grantedActionScopes" .
echo "=== requiredScopes usage (action gating) ==="
rg -n --hidden --glob '!**/.build/**' "requiredScopes" Packages/CmuxExtensionKit/Sources .
echo "=== Search for rejection / granted scope checks ==="
rg -n --hidden --glob '!**/.build/**' "(grantedActionScopes|grantedReadScopes).*contains|contains.*(grantedActionScopes|grantedReadScopes)|(reject|rejected).*(scope|granted)|requiredScopes.*granted" Packages/CmuxExtensionKit/Sources Packages/CMUXExtensionHostSupport .Repository: manaflow-ai/cmux
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Inspect action gating in CMUXInstalledExtensionSidebarHostView.swift (around 1284) ==="
HOSTVIEW="Sources/CMUXInstalledExtensionSidebarHostView.swift"
# Print a small window around the known match
start=1250
end=1320
sed -n "${start},${end}p" "$HOSTVIEW" | cat -n
echo "=== CMUXSidebarAction.swift: requiredScopes ==="
ACTIONFILE="Packages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarAction.swift"
sed -n '1,120p' "$ACTIONFILE" | cat -n
echo "=== Narrow searches for granted/allowed/required scopes (Swift only) ==="
rg -n --type=swift --hidden --glob '!**/.build/**' \
"(allowedActionScopes|requiredScopes|grantedActionScopes|grantedReadScopes|isSuperset\\(of: action\\.requiredScopes\\))" \
Packages/CmuxExtensionKit/Sources Sources --max-count=200
echo "=== Search for decodeLossySetIfPresent outside CMUXSidebarSnapshot.swift ==="
rg -n --type=swift --hidden --glob '!**/.build/**' "decodeLossySetIfPresent" Packages/CmuxExtensionKit/Sources --max-count=50Repository: manaflow-ai/cmux
Length of output: 10849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TESTFILE="Packages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swift"
echo "=== Search tests for 'lossy' or 'unknown' ==="
rg -n "lossy|unknown|rawValue|compactMap" "$TESTFILE"
echo "=== Show around decoded/grantedActionScopes expectations ==="
# show first 260 lines and then around the lines earlier hit (approx line numbers)
sed -n '1,120p' "$TESTFILE" | cat -n
sed -n '120,260p' "$TESTFILE" | cat -n
sed -n '260,420p' "$TESTFILE" | cat -nRepository: manaflow-ai/cmux
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TESTFILE="Packages/CmuxExtensionKit/Tests/CmuxExtensionKitTests/CmuxExtensionKitTests.swift"
echo "=== test file length ==="
wc -l "$TESTFILE"
echo "=== show beginning (1-120) ==="
sed -n '1,120p' "$TESTFILE" | cat -n
echo "=== show middle (120-240) ==="
sed -n '120,240p' "$TESTFILE" | cat -n
echo "=== show end (240-420) ==="
sed -n '240,420p' "$TESTFILE" | cat -nRepository: manaflow-ai/cmux
Length of output: 20413
Clarify lossy scope decoding as fail-closed forward-compat behavior
decodeLossySetIfPresentinPackages/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swiftdrops unknown scope raw strings viacompactMap(type.init(rawValue:)), so older extensions treat newly-added scopes as “not granted” (no permission expansion).- Host-side action enforcement in
Sources/CMUXInstalledExtensionSidebarHostView.swiftrejects any action whenallowedActionScopesisn’t a superset ofaction.requiredScopes, so the worst case is reduced UI/data for older extensions. - Add a brief comment/README note calling out this contract, and the
workspaceMetadata-missing behavior that sets workspacetitleto""inCMUXSidebarSnapshot.filtered(for:actionScopes:).
🤖 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/CmuxExtensionKit/Sources/CmuxExtensionKit/Sidebar/CMUXSidebarSnapshot.swift`
around lines 74 - 82, Add a short clarifying comment near the lossy decoding
util and a note in CMUXSidebarSnapshot explaining the fail-closed,
forward-compat behavior: reference
KeyedDecodingContainer.decodeLossySetIfPresent(Value.Type,forKey:) to state that
unknown scope raw values are dropped (treated as not granted), and mention that
host-side enforcement (in CMUXInstalledExtensionSidebarHostView) will reject
actions unless allowedActionScopes is a superset of action.requiredScopes; also
document the workspaceMetadata-missing behavior in
CMUXSidebarSnapshot.filtered(for:actionScopes:) that causes workspace.title to
be set to an empty string.
Comment on lines
+5
to
+9
| /// Handles a mutation against the latest sidebar snapshot. | ||
| func handle( | ||
| _ mutation: CmuxSidebarProviderMutation, | ||
| snapshot: CmuxSidebarProviderSnapshot | ||
| ) throws -> CmuxSidebarProviderCommandResult |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add - Parameter/- Returns/- Throws DocC callouts to handle(_:snapshot:).
This public protocol requirement takes two parameters, returns a result, and is throws, but only has a one-line summary. Document the mutation/snapshot inputs, the returned result, and the throwing conditions.
📝 Proposed docs
- /// Handles a mutation against the latest sidebar snapshot.
+ /// Handles a mutation against the latest sidebar snapshot.
+ ///
+ /// - Parameters:
+ /// - mutation: The mutation the host is requesting.
+ /// - snapshot: The latest sidebar snapshot the mutation applies to.
+ /// - Returns: The result indicating whether CMUX accepted the command.
+ /// - Throws: An error if the provider cannot apply the mutation.
func handle(
_ mutation: CmuxSidebarProviderMutation,
snapshot: CmuxSidebarProviderSnapshot
) throws -> CmuxSidebarProviderCommandResultAs per coding guidelines: "Document every public symbol in new Swift packages under Packages/ ... use - Parameter name:, - Returns:, - Throws: callouts."
🤖 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/CmuxSidebarProviderKit/Sources/CmuxSidebarProviderKit/Mutations/CmuxMutableSidebarProvider.swift`
around lines 5 - 9, Update the public protocol requirement declaration for
handle(_:snapshot:) to include DocC callouts: add a one-line summary plus "-
Parameter mutation: CmuxSidebarProviderMutation — description of the mutation
input", "- Parameter snapshot: CmuxSidebarProviderSnapshot — description of the
snapshot input", "- Returns: CmuxSidebarProviderCommandResult — describe what
the result represents", and "- Throws: — describe under what conditions the
method throws"; target the protocol method named handle(_:snapshot:) in
CmuxMutableSidebarProvider and reference the types CmuxSidebarProviderMutation,
CmuxSidebarProviderSnapshot, and CmuxSidebarProviderCommandResult so the
documentation appears for this public symbol.
azooz2003-bit
dismissed
coderabbitai[bot]’s stale review
June 2, 2026 15:24
Dismissed for merge: bot review is blocking on broad/nit comments after current-head build and release checks passed; remaining tests job is a stuck runner.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Split the extension SDK into a clean extension-author API and app-side provider/host layers, and moved the sidebar contract to API v2 with simpler manifests, typed action results, and connection status. Updated examples, added string catalogs, tightened async calls, and introduced a stub agent extension.
Refactors
CmuxSidebarProviderKitfor in-process providers with typed render models; examples now import it.CMUXExtensionClientwithCMUXExtensionHostSupport(host view + browser presenter); removed discovery/registry/session and tests.readScopes/actionScopes,CmuxExtensionAPIVersion.sidebarV2, renamedCMUX*→Cmux*(actions, surfaces, XPC codec, validator, extension point), and switched Info.plist key toCmuxSidebarExtensionPointIdentifier; ExtensionKit point is generated at build time.CmuxExtension,CmuxUIExtension) intoCmuxSidebarExtensionrefining ExtensionKit; action results now useCmuxSidebarActionResultwith optionalrejectionReason.New Features
.createWorkspaceWithPathaction scope with localized permission copy.CmuxSidebarConnectionStatusand asyncCmuxSidebarHostmethods; samples handle status and useString(localized:).moveWorkspace,openWorkspacePopover/window,openURL).Examples/StubAgentSidebarExtensionand.xcstringslocalizations across samples and the Tabs Visible example.Written for commit fad4790. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Documentation