Conversation
|
To use Codex here, create an environment for this repo. |
📝 WalkthroughWalkthroughAdds configurable full and MVP experience profiles for iOS. The selected policy propagates from build configuration through shell state, filters host capabilities, and gates browser, workspace, artifact, networking, and team-switching UI surfaces. ChangesMobile experience policy
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant BuildConfiguration
participant cmuxApp
participant CMUXMobileRootScene
participant CMUXMobileShellStore
participant MobileShellComposite
participant MobileExperiencePolicy
BuildConfiguration->>cmuxApp: provide CMUXExperienceProfile
cmuxApp->>MobileExperiencePolicy: create selected policy
cmuxApp->>CMUXMobileRootScene: pass experiencePolicy
CMUXMobileRootScene->>CMUXMobileShellStore: pass experiencePolicy
CMUXMobileShellStore->>MobileShellComposite: initialize policy
MobileShellComposite->>MobileExperiencePolicy: filter host capabilities
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperienceProfile.swift`:
- Around line 10-18: Update MobileExperienceProfile.init(configurationValue:) so
missing or malformed values fail closed by preserving an unavailable state or
selecting the restrictive .mvp policy for distribution builds, while retaining
an explicit development path for .full. In
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperienceProfileTests.swift
lines 10-14, replace .full fallback assertions with tests verifying restrictive
unavailable-profile behavior. In ios/README.md lines 37-39, document fail-closed
handling and the explicit development-profile configuration path.
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Line 1324: Require an explicit MobileExperiencePolicy when constructing
MobileShellComposite instead of defaulting omitted arguments to .full. Update
MobileShellComposite, MobileSettingsView settings construction, and
TerminalPickerMenuValue at
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift:1324-1324,
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swift:522-524,
and
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift:27-28
so production entry points pass the selected policy explicitly and no
constructor infers .full from missing input.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swift`:
- Around line 599-601: Update the workspace-group action closures
renameWorkspaceGroupClosure, setWorkspaceGroupPinnedClosure,
ungroupWorkspaceGroupClosure, and deleteWorkspaceGroupClosure to also enforce
store.experiencePolicy.allowsAdvancedWorkspaceManagement, returning nil or
otherwise failing closed when the policy is disabled, consistent with the
existing groups list gating.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift`:
- Around line 78-92: Extend firstReleasePolicyCanHideBothBrowserEntryPoints
beyond TerminalPickerMenuValue field assertions by testing the rendered
TerminalPickerMenu.menuContent, or extract its pure menu-item decision logic
into a testable helper. Assert that both the browser stream section and New
Browser action are absent when showsBrowserStreamSection and allowsLocalBrowser
are false.
🪄 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 Plus
Run ID: be60e8c1-f75d-4b8f-a89b-3a8c1c5cd918
📒 Files selected for processing (22)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperiencePolicy.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperienceProfile.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+Capabilities.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+MacUpdateHint.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+SecondaryPromotion.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperiencePolicyTests.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperienceProfileTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView+WorkspaceActions.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceShellView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swiftios/Config/Info.plistios/Config/Release.xcconfigios/Config/Shared.xcconfigios/README.mdios/cmux/cmuxApp.swiftios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swiftios/scripts/reload.sh
Comment on lines
+10
to
+18
| /// Creates a profile from an Info.plist or build-setting value. | ||
| /// | ||
| /// Unknown and missing values resolve to ``full`` so local development | ||
| /// never loses tools because of a malformed optional setting. | ||
| public init(configurationValue: String?) { | ||
| let normalized = configurationValue? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .lowercased() | ||
| self = normalized == Self.mvp.rawValue ? .mvp : .full |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Fail closed when the experience profile is unavailable.
A missing or malformed CMUXExperienceProfile currently selects .full. A Release/TestFlight configuration failure can then expose surfaces that the MVP policy must disable.
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperienceProfile.swift#L10-L18: Preserve invalid or missing input as an unavailable profile, or select the restrictive MVP policy for distribution builds.Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperienceProfileTests.swift#L10-L14: Replace the.fullfallback assertions with coverage that verifies restrictive behavior for unavailable distribution configuration.ios/README.md#L37-L39: Document the fail-closed behavior and the explicit development-profile path.
As per path instructions, required profile or capability data must fail closed and must not use guessed fallbacks that expose disabled MVP features.
📍 Affects 3 files
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperienceProfile.swift#L10-L18(this comment)Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperienceProfileTests.swift#L10-L14ios/README.md#L37-L39
🤖 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/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileExperienceProfile.swift`
around lines 10 - 18, Update MobileExperienceProfile.init(configurationValue:)
so missing or malformed values fail closed by preserving an unavailable state or
selecting the restrictive .mvp policy for distribution builds, while retaining
an explicit development path for .full. In
Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileExperienceProfileTests.swift
lines 10-14, replace .full fallback assertions with tests verifying restrictive
unavailable-profile behavior. In ios/README.md lines 37-39, document fail-closed
handling and the explicit development-profile configuration path.
Source: Path instructions
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
Testing
Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Add an iOS experience profile system and ship the focused
mvpprofile for the first release. It filters host capabilities and hides advanced UI to center on remote agent control.New Features
MobileExperienceProfile(full,mvp) andMobileExperiencePolicythat filter authenticated host capabilities and gate UI.mvp.mvp.CMUXExperienceProfileread from Info.plist (CMUX_IOS_EXPERIENCE_PROFILE);Release.xcconfigsetsmvp;reload.shadds--mvp; README documents the profile.Migration
full.mvp. To verify locally:ios/scripts/reload.sh --tag <tag> --mvpor setCMUX_IOS_EXPERIENCE_PROFILE=mvp.Written for commit ff5006c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes