Skip to content

Conversation

@pitzcarraldo
Copy link
Contributor

@pitzcarraldo pitzcarraldo commented Feb 3, 2026

Summary

Add comprehensive iOS APNS push notification setup with Firebase integration. Includes Apple account authentication, automated capability syncing, Notification Service Extension generation, and Firebase credential management.

Details

  • Apple Developer Portal integration for APNS key management and app capabilities
  • Firebase OAuth authentication for credential retrieval
  • Automated iOS project analysis and configuration
  • Notification Service Extension generation and Xcode project modification
  • P8 key validation and secure credential storage
  • Guided setup wizard for both push notifications and Firebase config

Related Issues

Related to #16 - OAuth credentials integration
Related to #9 - NSE setup guide

How to Validate

  1. Run bun run dev ios-setup and follow the setup flow
  2. Verify Apple account login works with both API Key and User credentials
  3. Check Firebase configuration detection for test projects
  4. Validate that Extension files are correctly generated
  5. Confirm all tests pass: bun test

Pre-Merge Checklist

Code Quality

  • Code builds without errors (bun run build)
  • Types check correctly (bun run typecheck)
  • Linter passes (bun run lint)
  • Tests pass (bun test)
  • Code cleanup: removed 198 lines of unused/duplicate code

Documentation

  • Updated SKILL.md files for iOS setup
  • Architecture documented in CLAUDE.md

Commit Standards

  • Commits follow Conventional Commits format
  • No breaking changes

Platform Validation

  • macOS

Summary by CodeRabbit

  • New Features

    • Multi‑phase iOS setup: automatic capability sync, guided Notification Service Extension scaffolding, optional APNS (.p8) key setup with Apple login/key management and P8 validation; interactive /ios-setup command and overlay with guided wizards for Xcode, Podfile, and Firebase upload.
  • Chores

    • Version bumped to 1.1.2‑beta.2; runtime dependencies updated.
  • Docs

    • README revised to describe phased, automated-guided iOS setup.
  • Tests

    • Added tests for APNS (.p8) validation utilities.

pitzcarraldo and others added 4 commits February 2, 2026 18:09
Add comprehensive push notification setup workflow:
- Create PushSetupWizard with P8 file detection and validation
- Add APNS key creation guide with Apple Developer Portal integration
- Implement Firebase OAuth authentication for project selection
- Auto-detect Team ID from Xcode project settings (DEVELOPMENT_TEAM)
- Create GuidedSetupWizard for NSE file generation guidance
- Integrate /ios-setup with /push-setup into unified IosSetupFlow
- Add P8 file auto-discovery in current directory
Replace AI agent-driven ios-setup with React UI wizard:
- Remove ios-setup SKILL.md and embedded skill definition
- Update documentation for new /ios-setup and /push-setup commands
- Simplify skills registry by removing ios-setup from embedded skills
- Add Apple account authentication with session restoration
  - Skip password if already authenticated
  - Use @expo/apple-utils for session management
  - Store credentials securely in macOS Keychain

- Add APNS key management features
  - Create new keys via Apple account
  - List existing keys with download status
  - Download keys (if canDownload is true)
  - Save P8 files to project directory

- Add pbxproj and Podfile manipulation modules
  - pbxproj-modifier for NSE target management
  - podfile-modifier for CocoaPods integration

- Fix bundling issues
  - Upgrade simple-plist to 1.4.0 (ESM compatible)
  - Add package overrides for transitive deps

- Update NotificationService template to use init() pattern
- Remove unused generateAgentPrompt() function (118 lines)
- Remove unused ApiKeyAuthConfigNew alias
- Remove unused validatePackageNameMatch/validateBundleIdMatch exports
- Remove unused _projectPath parameter from getExpectedPaths()
- Consolidate duplicate ApiKeyAuthConfig interface definition
- Extract common stdout/stderr handler in bash-service.ts

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

Walkthrough

Reworks iOS setup into a three‑phase, interactive CLI flow (capabilities sync, guided Notification Service Extension creation, optional APNS key setup) and adds Apple authentication, keychain support, pbxproj/Podfile modifiers, APNS key tooling, UI wizards/overlays, package updates, and tests.

Changes

Cohort / File(s) Summary
Docs & CLI counts
README.md, llms.txt
Rewrite iOS setup docs into three phases (Capabilities, Extension, APNS key); update slash/system command counts and related wording.
Package & build
package.json, scripts/build.ts
Bump version to 1.1.2-beta.2; add deps keychain, simple-plist, xcode; pin [email protected]; mark @expo/plist as external in build.
Command registration & CLI
src/cli.tsx, src/lib/commands/ios-setup.tsx, src/lib/commands/registry.ts, src/lib/commands/skills.ts
Rename helper to runIosSetupCommand, add iosSetupCommand (local-jsx) with aliases, remove ios-setup from generated skill list and register new command.
Orchestrator & core flow
src/commands/ios-setup/index.tsx
Replace single-step agent flow with orchestrated phases: direct setup, ProjectModification (automated NSE/pbxproj/Podfile edits), GuidedSetup fallback, and PushSetup; add UI components, print helpers, and consolidated outputs.
Apple auth & keychain
src/lib/ios/apple-auth.ts, src/lib/ios/keychain.ts
Add API-key and user/password Apple auth, session handling, credential caching, keychain integration with CLIX_NO_KEYCHAIN, and related helpers/types.
APNS / push tooling
src/lib/ios/push-key.ts, src/lib/push/*
New push tooling: APNS key list/create/download/revoke, P8 validation, constants for Apple/Firebase steps, types, tests, and index aggregator.
Xcode / Podfile automation
src/lib/ios/pbxproj-modifier.ts, src/lib/ios/podfile-modifier.ts, src/lib/ios/project-analyzer.ts, src/lib/ios/entitlements-manager.ts
Add .pbxproj modifier to add NSE targets/build settings; Podfile modifier to add extension target; project analyzer extracts teamId; entitlements typing adjustments.
NSE scaffolding & templates
src/lib/ios/extension-generator.ts, src/lib/ios/extension-templates.ts
New extension generator and templates to scaffold NotificationService.swift, Info.plist, .entitlements, and a Podfile snippet generator.
Public exports & types
src/lib/ios/index.ts, src/types/xcode.d.ts, src/types/keychain.d.ts
Expand ios public exports (auth, push, pbxproj/podfile modifiers, extension tools); remove generateAgentPrompt export; add type declarations for xcode and keychain.
Removed agent skill & prompt
src/lib/ios/agent-prompt-generator.ts, src/lib/embedded-skills.ts, src/lib/skills.ts, src/lib/skills/ios-setup/SKILL.md
Remove autonomous agent iOS setup prompt/skill and large SKILL.md; move NSE registration sample to init and simplify embedded skill content.
Firebase updates
src/lib/services/firebase/...
Drop unused projectPath arg from getExpectedPaths, update callers; add optional TEAM_ID to GoogleServiceInfoPlist; remove two validator exports.
Bash output handling
src/lib/services/bash-service.ts
Consolidate stdout/stderr handlers into a shared data handler factory to unify truncation/accumulation logic.
UI: setup flows & components
src/ui/IosSetupUI.tsx, src/ui/components/*
Add Ink-based UIs: IosSetupFlow orchestrator, enhanced IosSetupUI, GuidedSetupWizard (NSE steps), AppleLoginUI (Apple auth/key ops), PushSetupWizard (p8 handling & Firebase upload).
Chat integration & overlays
src/ui/chat/ChatApp.tsx, src/ui/chat/hooks/*
Add ios-setup overlay, command wiring to open it, and overlay completion handler that emits a system message.
Tests & misc UI
src/lib/commands/__tests__/registry.test.ts, src/lib/push/__tests__/p8-validator.test.ts, src/ui/components/FirebaseWizard.tsx
Add registry test for ios-setup, P8 validator tests; FirebaseWizard now uses shared openBrowser helper.

Possibly related PRs

Suggested reviewers

  • JeongwooYoo
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main feature added - iOS APNS and Firebase setup integration - which is the primary objective of this comprehensive changeset.
Description check ✅ Passed The description follows the template with all required sections (Summary, Details, Related Issues, How to Validate, Pre-Merge Checklist) properly completed and marked.
Docstring Coverage ✅ Passed Docstring coverage is 95.88% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/apns-firebase-setup

Comment @coderabbitai help to get the list of available commands and usage tips.

@pitzcarraldo pitzcarraldo self-assigned this Feb 3, 2026
@pitzcarraldo pitzcarraldo marked this pull request as ready for review February 3, 2026 08:10
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed120af5d8

ℹ️ 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".

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Fix all issues with AI agents
In `@llms.txt`:
- Around line 12-13: Add a high-level "Command Summary" section to README.md
that mirrors the llms.txt summary (lines referenced in the review) showing the
20 slash commands broken down as "4 autonomous commands + 5 interactive skills +
11 system commands"; ensure the section explicitly lists the three categories
and the total count and notes that /ios-setup remains in README (no change) so
readers can find the detailed setup separately, and then update llms.txt to
match if its summary differs—modify the README.md "Command Summary" heading and
content and the llms.txt summary text so both files contain the same category
names and counts.

In `@src/commands/ios-setup/index.tsx`:
- Around line 181-299: The extension push environment is hard-coded and failures
in createExtensionFiles don't stop the flow; update executeProjectModification
to set extContext.pushEnvironment from the incoming requested value (use
directResult.requestedPushEnvironment or a new parameter if that field doesn't
exist, falling back to 'development') and, if createExtensionFiles returns
!success, mark result.success = false, result.requiresManualSteps = true, push
the error into result.warnings (or result.error) and immediately return to avoid
continuing with pbxproj/Podfile changes; also ensure runIosSetupCommand /
runProjectModification passes the user's options.pushEnvironment into the
runProjectModification/directResult so the requested environment is threaded
through.

In `@src/lib/commands/ios-setup.tsx`:
- Around line 27-35: The call method currently awaits runIosSetupCommand without
error handling so onDone may never be invoked; wrap the await in a try-catch
inside the call function, call onDone('iOS setup complete') on success, and in
the catch call onDone with an error state or message (e.g., onDone?.(`iOS setup
failed: ${err.message}`)), and also log the error (or rethrow if appropriate) so
the UI is always notified; reference the call method and runIosSetupCommand to
locate where to add the try-catch.

In `@src/lib/ios/extension-templates.ts`:
- Around line 53-56: The Podfile snippet generator generatePodfileSnippet
currently interpolates extensionName directly into a single-quoted Ruby target
which breaks if extensionName contains an apostrophe; update
generatePodfileSnippet to escape apostrophes in extensionName (replace each '
with \' in the string) before embedding it into the template so the resulting
Ruby single-quoted target remains valid.

In `@src/lib/ios/keychain.ts`:
- Around line 58-107: getPasswordAsync and setPasswordAsync currently ignore the
CLIX_NO_KEYCHAIN flag; update both functions to short-circuit when
CLIX_NO_KEYCHAIN is enabled (same as the non-mac check) so they do not interact
with the OS keychain: in getPasswordAsync return null immediately if
CLIX_NO_KEYCHAIN is true, and in setPasswordAsync return Promise.resolve(false)
immediately if CLIX_NO_KEYCHAIN is true; modify the top of each function (around
IS_MAC checks) to include the CLIX_NO_KEYCHAIN guard so keychain.getPassword /
keychain.setPassword are never called when that flag is set.

In `@src/lib/ios/pbxproj-modifier.ts`:
- Around line 182-192: Replace the unreliable use of project.getFirstTarget() in
the embedding logic by selecting the app target filtered with productType ===
'com.apple.product-type.application' (and honor an optional mainTargetName in
PbxprojModifierOptions to pick a specific app target); after locating the
correct app target, call project.addTargetDependency(appTarget.uuid,
[target.uuid]) as before and then create the required "Embed App Extensions"
copy-files build phase with project.addBuildPhase(appTarget.uuid,
'PBXCopyFilesBuildPhase', 'Embed App Extensions', '10') (or equivalent API) so
the extension is actually copied into the app bundle; update
PbxprojModifierOptions to accept mainTargetName and use that when resolving the
app target.
- Around line 155-166: The build-settings block uses
`${options.extensionName}/...` literals causing wrong paths when
options.extensionDir is different; update the buildSettings (the
CODE_SIGN_ENTITLEMENTS and INFOPLIST_FILE entries) to derive project-relative
paths from options.extensionDir (the same way earlier code derives paths for the
target) instead of using options.extensionName, ensuring INFOPLIST_FILE and
CODE_SIGN_ENTITLEMENTS reference the computed extensionDir path; keep other keys
(ENABLE_USER_SCRIPT_SANDBOXING, PRODUCT_BUNDLE_IDENTIFIER,
IPHONEOS_DEPLOYMENT_TARGET, SWIFT_VERSION, TARGETED_DEVICE_FAMILY,
GENERATE_INFOPLIST_FILE) unchanged.

In `@src/lib/ios/push-key.ts`:
- Around line 94-123: When creating an APNS key with Keys.createKeyAsync then
downloading it with Keys.downloadKeyAsync, ensure you wrap the download call in
its own try-catch so that on any download failure you revoke the newly created
key (e.g. call Keys.revokeKeyAsync or the repository's key-deletion API with the
same context and key.id) before rethrowing the original error; update the block
around Keys.createKeyAsync and Keys.downloadKeyAsync to await the revoke call
inside the download catch, passing the same context and key.id so the failed key
doesn’t consume one of the two APNS slots.

In `@src/ui/components/IosSetupFlow.tsx`:
- Around line 224-232: The code in IosSetupFlow currently calls
setPhase('push_confirm') directly during render when getGuidedSetupContext()
returns null; move this imperative state change into a useEffect: add a
useEffect that watches phase and the guided context (e.g.,
directResult?.agentContext or result of getGuidedSetupContext) and if phase ===
'guided_setup' and context is missing call setPhase('push_confirm'), then remove
the inline setPhase call from the render branch (leave the branch to return null
or the Wizard JSX only). Ensure you reference getGuidedSetupContext, phase, and
setPhase in the effect dependencies so the state transition happens safely
outside render.

In `@src/ui/components/PushSetupWizard.tsx`:
- Around line 1040-1065: The current sync write may throw if both writes fail;
wrap the write logic for p8FilePath and fallbackPath in try/catch so any thrown
error is caught, log or capture the Error, and call setContext to transition the
wizard into an error phase (populate context.error or similar) instead of
letting the UI crash; update the code that sets pushKey and p8FilePath
(references: p8FileName, p8FilePath, fallbackPath, result.pushKey, and
setContext) to only run on success, and in the catch handler setContext with an
error state/message describing the file write failure so the UI can render the
error phase.
- Around line 742-744: The displayed hint Text inside PushSetupWizard.tsx
currently references a non-existent "/push-setup" command; update the Text
(inside the Box/Text JSX in PushSetupWizard) to either remove the command
suggestion entirely or replace "/push-setup" with the valid setup command (e.g.,
"/ios-setup") after confirming that is the correct action for push notification
configuration, and ensure the string content is updated accordingly.
🧹 Nitpick comments (4)
src/ui/components/FirebaseWizard.tsx (1)

970-973: Consider logging swallowed errors for debugging.

The empty catch block silently swallows errors when fetching iOS apps. While the fallback behavior (downloading Android config only) is reasonable and documented with a comment, logging the error would aid debugging when iOS app fetching unexpectedly fails.

🔧 Suggested improvement
-      } catch {
+      } catch (err) {
         // Failed to get iOS apps, just download Android
+        console.debug('Failed to fetch iOS apps, proceeding with Android only:', err);
         downloadConfigsRef.current(project, app, null);
       }
src/lib/ios/project-analyzer.ts (1)

198-214: Consider simplifying redundant regex patterns.

The two patterns in extractTeamId are functionally redundant. Pattern 1 with "? already handles both quoted and unquoted values:

 function extractTeamId(pbxprojContent: string): string | undefined {
-  // Pattern: DEVELOPMENT_TEAM = XXXXXXXXXX;
-  // or: DEVELOPMENT_TEAM = "XXXXXXXXXX";
-  const patterns = [
-    /DEVELOPMENT_TEAM\s*=\s*"?([A-Z0-9]{10})"?\s*;/,
-    /DEVELOPMENT_TEAM\s*=\s*([A-Z0-9]{10})\s*;/,
-  ];
-
-  for (const pattern of patterns) {
-    const match = pbxprojContent.match(pattern);
-    if (match?.[1]) {
-      return match[1];
-    }
-  }
+  // Pattern: DEVELOPMENT_TEAM = XXXXXXXXXX; or DEVELOPMENT_TEAM = "XXXXXXXXXX";
+  const match = pbxprojContent.match(/DEVELOPMENT_TEAM\s*=\s*"?([A-Z0-9]{10})"?\s*;/);
+  if (match?.[1]) {
+    return match[1];
+  }
 
   return undefined;
 }
src/commands/ios-setup/index.tsx (1)

390-407: Pass the pre-detected Team ID into the push wizard.

This keeps the CLI flow aligned with the UI flow and reduces re-prompting.

💡 Suggested tweak
       <PushSetupWizard
         projectPath={projectPath}
         preDetectedBundleId={directResult.bundleId}
         preDetectedFirebaseProjectId={directResult.firebaseProjectId}
+        preDetectedTeamId={directResult.teamId ?? undefined}
         onComplete={(result) => {
src/lib/push/p8-validator.ts (1)

26-38: Consider handling missing HOME environment variable more robustly.

If process.env.HOME is undefined (possible on some Windows configurations), the path expansion on line 29 would result in an invalid path starting with /. This may not be a practical concern if the tool is macOS-focused, but a more defensive approach would help:

♻️ Optional: Add explicit handling for undefined HOME
   // Expand ~ to home directory
-  const expandedPath = filePath.startsWith('~')
-    ? path.join(process.env.HOME || '', filePath.slice(1))
-    : filePath;
+  let expandedPath = filePath;
+  if (filePath.startsWith('~')) {
+    const home = process.env.HOME;
+    if (!home) {
+      return {
+        valid: false,
+        error: 'Cannot expand ~ path: HOME environment variable is not set',
+      };
+    }
+    expandedPath = path.join(home, filePath.slice(1));
+  }

- Propagate pushEnvironment to extension entitlements (fix production APNS)
- Add early return when extension file creation fails
- Revoke APNS key on download failure to avoid wasting limited slots
- Respect CLIX_NO_KEYCHAIN env var in getPasswordAsync/setPasswordAsync
- Handle P8 file write failures gracefully with error phase
- Add try-catch error handling to ios-setup command
- Escape apostrophes in Podfile extensionName for Ruby compatibility
- Compute relative paths from extensionDir in pbxproj build settings
- Filter app target by productType instead of getFirstTarget()
- Fix Firebase upload phase when selectedProject is null
- Move setPhase call to useEffect to avoid React anti-pattern
- Fix invalid /push-setup command reference to /ios-setup

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/ui/components/PushSetupWizard.tsx`:
- Around line 960-1008: The async authenticateAndFetchProjects inside the
useEffect can continue after the user cancels (handleCancel) and mutate state
for phase, projects, selectedProject, and error; add a local let cancelled =
false flag in the effect closure, check if (cancelled) return before any
setProjects/setSelectedProject/setPhase/setError calls inside
authenticateAndFetchProjects, and in the effect cleanup set cancelled = true so
pending FirebaseDownloader.authenticate/listProjects promise handlers no-op when
the phase has changed (ensure you still preserve creation of
downloaderRef.current and calls to FirebaseDownloader methods but guard their
state updates).
🧹 Nitpick comments (1)
src/commands/ios-setup/index.tsx (1)

394-411: Pass through Team ID to prefill push setup.
You already have the team ID on IosSetupResult; wiring it into the wizard will reduce manual input.

♻️ Suggested change
   return new Promise((resolve) => {
     const { unmount } = render(
       <PushSetupWizard
         projectPath={projectPath}
         preDetectedBundleId={directResult.bundleId}
         preDetectedFirebaseProjectId={directResult.firebaseProjectId}
+        preDetectedTeamId={directResult.teamId}
         onComplete={(result) => {
           unmount();
           resolve(result);
         }}

Add cancelled flag to Firebase authentication useEffect to prevent
stale state mutations when user cancels during async operation.
Extracted handleProjectSelection helper to reduce complexity.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/ui/components/PushSetupWizard.tsx`:
- Around line 632-669: The effect that opens the browser (useEffect referencing
browserOpened, selectedProject, context.firebaseProjectId) should handle the
case where both selectedProject and context.firebaseProjectId are null by
falling back to a generic console URL (PUSH_SETUP_URLS.firebaseConsole() or a
PUSH_SETUP_URLS.genericConsole constant) and calling openBrowser(...) and
setBrowserOpened(true) so the UI and state remain consistent; also update the
rendered project label where selectedProject?.displayName ||
selectedProject?.projectId is used to show a clear fallback like "Unknown
project" when neither value exists so the text isn't empty.

Open generic Firebase console when no project ID is known and display
"Unknown" label instead of empty text.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/ui/components/PushSetupWizard.tsx`:
- Around line 895-1231: PushSetupWizard is too large/complex; extract
responsibilities into focused hooks and a phase->component map: move the initial
detection effect into a usePushDetection(projectPath, preDetectedBundleId,
preDetectedFirebaseProjectId, preDetectedTeamId) hook that returns {phase,
setPhase, context, setContext, detectedTeamId}, move the Firebase auth/listing
logic into a useFirebaseAuth(phase, context.firebaseProjectId, downloaderRef)
hook that handles authenticateAndFetchProjects, projects, selectedProject and
setPhase, and move key handling logic (handleAppleLoginSuccess, handleP8Submit,
handleHasKey/NoKey, handleAppleGuideComplete, handleP8Submit) into a
usePushKeyHandlers(projectPath, isOAuthConfigured) hook that updates context and
decides next phase; finally replace the big switch with a phaseComponentMap
mapping each PushSetupPhase to a small component wrapper that receives necessary
props (context, projects, handlers) so PushSetupWizard becomes a thin
orchestrator wiring these hooks and rendering phaseComponentMap[phase].
- Around line 1056-1077: The APNS .p8 file writes in handleAppleLoginSuccess use
default permissions which can be world-readable; update the fs.writeFileSync
calls for p8FilePath and the fallbackPath (where p8FileName is used) to include
a file mode of 0o600 (and keep the 'utf-8' encoding) so the written key is
readable/writable only by the current user; ensure both the primary write
(p8FilePath) and the fallback write (fallbackPath) pass the options object ({
encoding: 'utf-8', mode: 0o600 }) to fs.writeFileSync.

Use mode 0o600 when writing .p8 files to prevent world-readable
sensitive cryptographic credentials.

Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/ui/components/PushSetupWizard.tsx`:
- Around line 920-964: The detect() async effect in PushSetupWizard lacks a
cancellation guard, so its async results may call setContext, setDetectedTeamId,
and setPhase after the component unmounts or the phase changes; add a local
`cancelled` (or `isMounted`) boolean in the useEffect, set it to true in the
cleanup, and check `if (cancelled) return` (or guard each state update with `if
(!cancelled)`) before calling setDetectedTeamId, setContext, or setPhase inside
the detect() function (referencing detect(), setContext, setDetectedTeamId,
setPhase, and the useEffect dependency list) so the effect mirrors the
cancellation behavior used in the Firebase auth effect.
🧹 Nitpick comments (2)
src/ui/components/PushSetupWizard.tsx (2)

240-244: Use map's index parameter instead of indexOf.

Using indexOf inside the map callback results in O(n²) complexity. The map callback already provides the index as its second parameter.

♻️ Proposed fix
-        {APNS_KEY_CREATION_STEPS.map((step) => (
-          <Text key={step} dimColor>
-            {APNS_KEY_CREATION_STEPS.indexOf(step) + 1}. {step}
+        {APNS_KEY_CREATION_STEPS.map((step, index) => (
+          <Text key={index} dimColor>
+            {index + 1}. {step}
           </Text>
         ))}

711-715: Use map's index parameter instead of indexOf.

Same issue as in AppleGuidePhase - using indexOf inside the map callback is inefficient.

♻️ Proposed fix
-        {FIREBASE_UPLOAD_STEPS.map((step) => (
-          <Text key={step} dimColor>
-            {FIREBASE_UPLOAD_STEPS.indexOf(step) + 1}. {step}
+        {FIREBASE_UPLOAD_STEPS.map((step, index) => (
+          <Text key={index} dimColor>
+            {index + 1}. {step}
           </Text>
         ))}

Comment on lines +920 to +964
useEffect(() => {
const detect = async () => {
// Use pre-detected values if available (from ios-setup integration)
if (preDetectedBundleId !== undefined || preDetectedFirebaseProjectId !== undefined) {
setContext((prev) => ({
...prev,
firebaseProjectId: preDetectedFirebaseProjectId ?? null,
bundleId: preDetectedBundleId ?? null,
}));
if (preDetectedTeamId) {
setDetectedTeamId(preDetectedTeamId);
}
setPhase('status');
return;
}

// Detect from Firebase config
const firebaseResult = await detectFromFirebase(projectPath);
let { firebaseProjectId, bundleId, teamId } = firebaseResult;

// Try Xcode project if Team ID not found in Firebase config
if (!teamId) {
const xcodeResult = await detectFromXcodeProject(projectPath);
teamId = xcodeResult.teamId;
if (!bundleId) {
bundleId = xcodeResult.bundleId;
}
}

if (teamId) {
setDetectedTeamId(teamId);
}

setContext((prev) => ({
...prev,
firebaseProjectId,
bundleId,
}));
setPhase('status');
};

if (phase === 'detecting') {
detect();
}
}, [phase, projectPath, preDetectedBundleId, preDetectedFirebaseProjectId, preDetectedTeamId]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add cancellation guard to the initial detection effect.

The async detect() function lacks a cancellation guard. If the user cancels during detection, the pending async operations continue and call setContext, setDetectedTeamId, and setPhase after the phase has changed or the component has unmounted. This mirrors the issue that was fixed in the Firebase auth effect (Lines 967-1025).

🛡️ Proposed fix
   useEffect(() => {
+    let cancelled = false;
+
     const detect = async () => {
       // Use pre-detected values if available (from ios-setup integration)
       if (preDetectedBundleId !== undefined || preDetectedFirebaseProjectId !== undefined) {
+        if (cancelled) return;
         setContext((prev) => ({
           ...prev,
           firebaseProjectId: preDetectedFirebaseProjectId ?? null,
           bundleId: preDetectedBundleId ?? null,
         }));
         if (preDetectedTeamId) {
           setDetectedTeamId(preDetectedTeamId);
         }
         setPhase('status');
         return;
       }

       // Detect from Firebase config
       const firebaseResult = await detectFromFirebase(projectPath);
+      if (cancelled) return;
       let { firebaseProjectId, bundleId, teamId } = firebaseResult;

       // Try Xcode project if Team ID not found in Firebase config
       if (!teamId) {
         const xcodeResult = await detectFromXcodeProject(projectPath);
+        if (cancelled) return;
         teamId = xcodeResult.teamId;
         if (!bundleId) {
           bundleId = xcodeResult.bundleId;
         }
       }

       if (teamId) {
         setDetectedTeamId(teamId);
       }

       setContext((prev) => ({
         ...prev,
         firebaseProjectId,
         bundleId,
       }));
       setPhase('status');
     };

     if (phase === 'detecting') {
       detect();
     }
+
+    return () => {
+      cancelled = true;
+    };
   }, [phase, projectPath, preDetectedBundleId, preDetectedFirebaseProjectId, preDetectedTeamId]);
📝 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
useEffect(() => {
const detect = async () => {
// Use pre-detected values if available (from ios-setup integration)
if (preDetectedBundleId !== undefined || preDetectedFirebaseProjectId !== undefined) {
setContext((prev) => ({
...prev,
firebaseProjectId: preDetectedFirebaseProjectId ?? null,
bundleId: preDetectedBundleId ?? null,
}));
if (preDetectedTeamId) {
setDetectedTeamId(preDetectedTeamId);
}
setPhase('status');
return;
}
// Detect from Firebase config
const firebaseResult = await detectFromFirebase(projectPath);
let { firebaseProjectId, bundleId, teamId } = firebaseResult;
// Try Xcode project if Team ID not found in Firebase config
if (!teamId) {
const xcodeResult = await detectFromXcodeProject(projectPath);
teamId = xcodeResult.teamId;
if (!bundleId) {
bundleId = xcodeResult.bundleId;
}
}
if (teamId) {
setDetectedTeamId(teamId);
}
setContext((prev) => ({
...prev,
firebaseProjectId,
bundleId,
}));
setPhase('status');
};
if (phase === 'detecting') {
detect();
}
}, [phase, projectPath, preDetectedBundleId, preDetectedFirebaseProjectId, preDetectedTeamId]);
useEffect(() => {
let cancelled = false;
const detect = async () => {
// Use pre-detected values if available (from ios-setup integration)
if (preDetectedBundleId !== undefined || preDetectedFirebaseProjectId !== undefined) {
if (cancelled) return;
setContext((prev) => ({
...prev,
firebaseProjectId: preDetectedFirebaseProjectId ?? null,
bundleId: preDetectedBundleId ?? null,
}));
if (preDetectedTeamId) {
setDetectedTeamId(preDetectedTeamId);
}
setPhase('status');
return;
}
// Detect from Firebase config
const firebaseResult = await detectFromFirebase(projectPath);
if (cancelled) return;
let { firebaseProjectId, bundleId, teamId } = firebaseResult;
// Try Xcode project if Team ID not found in Firebase config
if (!teamId) {
const xcodeResult = await detectFromXcodeProject(projectPath);
if (cancelled) return;
teamId = xcodeResult.teamId;
if (!bundleId) {
bundleId = xcodeResult.bundleId;
}
}
if (teamId) {
setDetectedTeamId(teamId);
}
setContext((prev) => ({
...prev,
firebaseProjectId,
bundleId,
}));
setPhase('status');
};
if (phase === 'detecting') {
detect();
}
return () => {
cancelled = true;
};
}, [phase, projectPath, preDetectedBundleId, preDetectedFirebaseProjectId, preDetectedTeamId]);
🤖 Prompt for AI Agents
In `@src/ui/components/PushSetupWizard.tsx` around lines 920 - 964, The detect()
async effect in PushSetupWizard lacks a cancellation guard, so its async results
may call setContext, setDetectedTeamId, and setPhase after the component
unmounts or the phase changes; add a local `cancelled` (or `isMounted`) boolean
in the useEffect, set it to true in the cleanup, and check `if (cancelled)
return` (or guard each state update with `if (!cancelled)`) before calling
setDetectedTeamId, setContext, or setPhase inside the detect() function
(referencing detect(), setContext, setDetectedTeamId, setPhase, and the
useEffect dependency list) so the effect mirrors the cancellation behavior used
in the Firebase auth effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants