feat: add iOS setup autonomous command #4
Conversation
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
Adds /ios-setup command to configure iOS capabilities for Clix SDK: - Analyzes iOS project structure - Syncs Push Notifications and App Groups with Apple Developer Portal - Creates/updates entitlements files - Generates agent prompt for remaining Xcode project modifications Also fixes code review issues: removes unnecessary async keywords and improves error handling. Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new CLI command and local skill "ios-setup" to configure iOS Push Notifications and App Groups; implements project analysis, entitlements management, Apple Developer Portal integration, an Ink-based multi‑phase UI, agent prompt generation, and CLI routing and docs updates. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4f983d73d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
llms.txt (1)
79-80: Documentation inconsistency: command counts don't match.Line 79-80 states "17 slash commands (3 autonomous + 5 skills + 9 system)" but the updated counts elsewhere indicate 18 slash commands with 4 autonomous commands. This should be updated for consistency.
📝 Suggested fix
-Available within chat: -- All 17 slash commands (3 autonomous + 5 skills + 9 system) +Available within chat: +- All 18 slash commands (4 autonomous + 5 skills + 9 system)
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 50-51: The package.json lists "@expo/plist": "^0.2.0" which is
outdated; update that dependency to "@expo/plist": "^0.4.8" in package.json so
the project uses the latest 0.4.x release; after changing the version string,
run your package manager (npm/yarn/pnpm) to install and verify there are no
breaking changes affecting any code that imports `@expo/plist` (search for imports
of "@expo/plist" to validate usage).
In `@src/cli.tsx`:
- Around line 172-183: The code casts cli.flags.pushEnv to 'development' |
'production' without validation, risking invalid aps-environment values; update
the handling around the pushEnv local and the call to iosSetupCommand so you
first validate cli.flags.pushEnv is exactly "development" or "production"
(otherwise set pushEnv to undefined or throw a clear error), and then pass that
validated value into iosSetupCommand (references: variable pushEnv, input
cli.flags.pushEnv, and function iosSetupCommand) to ensure only allowed values
are forwarded.
In `@src/commands/ios-setup/index.tsx`:
- Around line 24-51: The current toDirectSetupOutput(IosSetupResult) always says
"Starting agent for remaining tasks..." even when result.agentContext is absent;
change the returned message (and optionally title) to be conditional on
result.agentContext presence: if result.agentContext exists keep the current
message, otherwise use a direct-only message like "Portal sync and entitlements
configured." (no agent-start text); update the return object in
toDirectSetupOutput to choose the appropriate message based on
result.agentContext while leaving details and type ('success') unchanged.
In `@src/lib/ios/apple-portal.ts`:
- Around line 145-221: The updateBundleIdCapabilityAsync usage is inconsistent
and may overwrite existing APP_GROUP relationships; verify whether
updateBundleIdCapabilityAsync accepts both a single capability object and an
array and whether APP_GROUP relationships are merged or replaced, and then make
the call consistent: use bundleId.getBundleIdCapabilitiesAsync to read existing
APP_GROUP relationships (find the capability where
cap.isType(CapabilityType.APP_GROUP)), merge existing app group IDs with
appGroup.id if the API replaces relationships, and then call
updateBundleIdCapabilityAsync with the properly merged payload (use the array
form if that's the documented contract); remove the silent try-catch around the
second update and instead handle/report errors when
updateBundleIdCapabilityAsync (for CapabilityType.APP_GROUP) fails so we don't
silently drop relationships.
In `@src/lib/ios/project-analyzer.ts`:
- Around line 260-281: The getIosProjectDir function inconsistently only checks
for an 'ios' subdirectory; update it to mirror
findXcodeProject/findXcodeWorkspace by checking both 'ios' and 'iOS' (or iterate
over ['ios','iOS'] candidates) when scanning the subdirectory for .xcodeproj
entries; modify the getIosProjectDir(cwd: string) logic to loop through both
directory name variants and return the matching path so behavior is consistent
on case-sensitive filesystems.
In `@src/lib/skills/ios-setup/SKILL.md`:
- Around line 45-92: Several fenced code blocks in SKILL.md are missing language
identifiers (markdownlint MD040); add appropriate language tags (e.g., ```text
or ```bash) to each fenced block so they render and lint correctly—specifically
update the fenced snippet under the "Report Current State" section and the
blocks inside the "**Add Push Notifications:**", "**Add Background Modes
(Recommended):**", and "**Add App Groups:**" headings (also apply the same
change to the other affected fences referenced around lines 133-180).
In `@src/ui/IosSetupUI.tsx`:
- Around line 169-189: The code sets result.success = true and completes even if
updateEntitlements returned null; change the flow so you only mark success and
set phase 'complete' when entitlementsResult is truthy (and files were updated),
e.g. after calling updateEntitlements check entitlementsResult and files before
assigning result.entitlementsUpdated and result.success, buildAgentContext only
when entitlementsResult exists, and otherwise set result.success = false, set an
error phase/state via setState (or leave phase indicating failure) and return
the result to avoid a false success; refer to updateEntitlements,
entitlementsResult, result.entitlementsUpdated, result.agentContext,
buildAgentContext, setState, and result.success to locate where to apply this
conditional logic.
- Around line 72-94: The syncWithPortal function currently treats partial
credential sets as a silent skip; change the logic to enforce all-or-none
validation: if options.skipPortal is true, return null immediately, otherwise
validate that options.apiKeyPath, options.keyId, and options.issuerId are all
present and if any are missing setPhase('error') (or similar) and throw a
descriptive error indicating which credential(s) are missing so callers see the
misconfiguration; keep the rest of the flow (loadApiKeyFromFile,
createAuthContext, syncCapabilities) unchanged and reference syncWithPortal,
options.skipPortal, options.apiKeyPath, options.keyId, options.issuerId, and
setPhase when implementing the check.
- Around line 105-131: The function updateEntitlements currently picks the main
target via project.targets[0]; instead, locate the primary app target
deterministically by checking for a target whose bundleId matches
project.bundleId, or whose productType indicates an application (e.g.,
"com.apple.product-type.application"), or that corresponds to an existing
entitlements file listed in project.entitlementsFiles; if multiple candidates
exist prefer the bundleId match, then entitlementsFiles match, then productType
fallback, and return null if no sensible app target is found. Update
updateEntitlements to compute mainTarget using that selection logic (referencing
project.targets, project.bundleId, project.entitlementsFiles and
getEntitlementsPath) before computing entitlementsPath and calling
updateEntitlementsForClix. Ensure the subsequent code still uses the chosen
mainTarget and handles the no-target-found case gracefully.
🧹 Nitpick comments (3)
src/lib/ios/project-analyzer.ts (2)
87-142: Consider extracting common logic to reduce duplication.
findXcodeProjectandfindXcodeWorkspacehave nearly identical implementations, differing only in the file extension they search for. This is acceptable given the small scope, but could be refactored to a shared helper if this pattern expands.
218-255: Remove unnecessary async/await - functions use only synchronous operations.Both
findEntitlementsFilesandsearchForEntitlementsare marked asasyncand useawait, but all filesystem operations inside are synchronous (fs.existsSync,fs.readdirSync). This works but is misleading. Either convert to sync functions or use async fs operations.♻️ Option 1: Convert to synchronous functions
-export async function findEntitlementsFiles(cwd: string): Promise<string[]> { +export function findEntitlementsFiles(cwd: string): string[] { const entitlementsFiles: string[] = []; const searchDirs = [cwd, path.join(cwd, 'ios'), path.join(cwd, 'iOS')]; for (const searchDir of searchDirs) { if (!fs.existsSync(searchDir)) continue; - await searchForEntitlements(searchDir, entitlementsFiles); + searchForEntitlements(searchDir, entitlementsFiles); } return entitlementsFiles; } -async function searchForEntitlements(dir: string, results: string[], depth = 0): Promise<void> { +function searchForEntitlements(dir: string, results: string[], depth = 0): void { // Limit search depth to avoid deep recursion if (depth > 5) return; // Skip certain directories const skipDirs = ['node_modules', 'Pods', 'build', 'DerivedData', '.git']; const entries = fs.readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { if (!skipDirs.includes(entry.name)) { - await searchForEntitlements(fullPath, results, depth + 1); + searchForEntitlements(fullPath, results, depth + 1); } } else if (entry.isFile() && entry.name.endsWith('.entitlements')) { results.push(fullPath); } } }Note: This will require updating the call site in
analyzeIosProject(line 69) to remove theawait.src/lib/ios/entitlements-manager.ts (1)
21-51: Async functions use synchronous filesystem operations.Similar to
project-analyzer.ts,readEntitlementsandwriteEntitlementsare markedasyncbut use synchronous filesystem operations (fs.existsSync,fs.readFileSync,fs.writeFileSync). This works but is misleading. Consider converting to sync or usingfs.promisesfor consistency.
- Fix iOS directory casing: getIosProjectDir now checks both ios and iOS - Add partial credentials validation with clear error message - Update @expo/plist to ^0.4.8 - Add pushEnv validation (development/production only) - Fix misleading agent message when agentContext is absent - Throw error when entitlements update fails instead of false success - Add language identifiers to SKILL.md code blocks Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Adds
/ios-setupcommand to configure iOS capabilities required for the Clix SDK, including Push Notifications and App Groups configuration with Apple Developer Portal integration.Details
/ios-setup(aliases:/capabilities,/ios-capabilities)How to Validate
clix ios-setup --helpto see command optionsclix ios-setupin an iOS project to start the configuration workflowbun run check && bun testPre-Merge Checklist
Summary by CodeRabbit
New Features
clix ios-setup(aliases:/capabilities,/ios-capabilities) to configure iOS push notifications and App Groups via a guided, multi‑phase setup that analyzes the project, updates entitlements, and optionally completes remaining steps.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.